Remove TextMate.selected_paths_array from shelltokenize.rb, but maybe keep functionality?

[noniq]

This is more or less a duplicate of TextMate.selected_files from textmate.rb. It is currently only used in a few places in the CVS, Mercurial, SVK and SVN bundles (sometimes via TextMate.selected_paths_for_shell).

However, there is one difference: If no files are selected, selected_path_array returns an array with a single entry for ENV["TM_FILEPATH"], or an empty array if this is not set. selected_files returns nil in those cases.

I wonder whether this functionality (“Give me an array of selected files, falling back to the current file if nothing is selected“) is so common that it should be part of the “official” bundle support API?

For example, the Git bundle has its own rather elaborate implementation that even supports different fallbacks (current file vs. project directory) and optional unique-filtering: https://github.com/textmate/git.tmbundle/blob/d1db42c2d71948662098183a6df519fb53a7a15b/Support/lib/git.rb#L114-L137

What do you think?

[sorbits]

I assume the library would be something like this:

require 'shellwords'

module TextMate
  def TextMate.file_selection
    ENV.has_key?('TM_SELECTED_FILES') ? Shellwords.shellwords(ENV['TM_SELECTED_FILES']) : nil
  end

  def TextMate.file_selection_or_current
    return Shellwords.shellwords(ENV['TM_SELECTED_FILES']) if ENV.has_key?('TM_SELECTED_FILES')
    return ENV.has_key?('TM_FILEPATH') ? [ ENV['TM_FILEPATH'] ] : nil
  end

  def TextMate.file_selection_or_project
    return Shellwords.shellwords(ENV['TM_SELECTED_FILES']) if ENV.has_key?('TM_SELECTED_FILES')
    folder = ENV['TM_PROJECT_DIRECTORY'] || ENV['TM_DIRECTORY']
    return folder ? [ folder ] : nil
  end
end

I could see a minor advantage of having commands use such library, even though inlining the relevant code is probably not that much more, but probably a bit more arcane.

I’m btw definitely in favor of returning nil over empty array, as the former can more easily be tested against.

[noniq]

Exactly. Btw, there is already TextMate.selected_files in textmate.rb:

bundle-support.tmbundle/Support/shared/lib/textmate.rb

Lines 213 to 217 in f656497

	def TextMate.selected_files( tm_selected_files = ENV['TM_SELECTED_FILES'] )
	return nil if tm_selected_files.nil? or tm_selected_files.empty?
	require 'shellwords'
	Shellwords.shellwords( tm_selected_files )
	end

. Maybe simply add an optional fallback: parameter so that it can be called like this:

TextMate.selected_files # default (nil if no files selected)
TextMate.selected_files(fallback: :current_file) 
TextMate.selected_files(fallback: :project_dir) 

On the other hand, having explicit methods would be more explicit. So not sure what’s the best way.

[sorbits]

I like having different method names for the different options,
especially with ruby where options are untyped (so one can pass in
virtually anything w/o getting errors).

Encoding the options in the method names also makes it easier to search
for the code which uses the options.

And given that we already have TextMate.selected_files, let’s
definitely move everything to moving that, instead of this shelltokenize
stuff.

The uniquing done by the Git bundle, maybe this was for 1.x projects
where the user could add the same file twice, but there should be no
reason to check for duplicates in TM_SELECTED_FILES with 2.0.

On 29 Aug 2016, at 23:17, Stefan Daschek wrote:

Exactly. Btw, there is already TextMate.selected_files in
textmate.rb:

bundle-support.tmbundle/Support/shared/lib/textmate.rb

Lines 213 to 217 in f656497

	def TextMate.selected_files( tm_selected_files = ENV['TM_SELECTED_FILES'] )
	return nil if tm_selected_files.nil? or tm_selected_files.empty?
	require 'shellwords'
	Shellwords.shellwords( tm_selected_files )
	end

.
Maybe simply add an optional fallback: parameter so that it can be
called like this:

TextMate.selected_files # default (nil if no files selected)
TextMate.selected_files(fallback: :current_file)
TextMate.selected_files(fallback: :project_dir)

On the other hand, having explicit methods would be more explicit. So
not sure what’s the best way.

-- 
You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
https://github.com/textmate/bundle-support.tmbundle/issues/25#issuecomment-243259362