Remove TextMate.selected_paths_array from shelltokenize.rb, but maybe keep functionality? about bundle-support.tmbundle HOT 3 OPEN

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.

from bundle-support.tmbundle.

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

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.

from bundle-support.tmbundle.

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

from bundle-support.tmbundle.