Use $EDITOR by default instead of Sublime Text

[dunn]

The $EDITOR variable is a well-established convention, and people using Sublime probably have it set to subl, so I think this is probably a better default setting.

[mattbrictson]

Thanks for the PR! This is a good idea, but maybe we should mimic how Rubygems does it for consistency with other Ruby tools:

https://github.com/rubygems/rubygems/blob/9051dd134a9278da1f41e9a8590e718ac18c77cf/lib/rubygems/commands/open_command.rb#L45-L50

ENV['GEM_EDITOR'] ||
  ENV['VISUAL'] ||
  ENV['EDITOR'] ||
  '/usr/local/bin/subl'

I think it is important to still provide a fallback in case $EDITOR is not set.

[dunn]

Oh, even better. I'll try to update the PR later this evening.

[dunn]

How does that look?

[mattbrictson]

Great! Now ideally we would have test coverage for this. Is that something you can take a stab at?

To make testing easier, it might be better to get rid of the DEFAULT_EDITOR constant and move the ENV logic into a private method. That way we can stub ENV[] in the spec (otherwise ENV is consulted only once, when the .rb file is parsed).

private

def default_editor
  # ENV access goes here...
end

def default_config
  { editor: default_editor } # instead of DEFAULT_EDITOR
end

Does that make sense?

[dunn]

Think so; I'll see what I can do.

[dunn]

How's that look?

[mattbrictson]

This is a good improvement, but I notice that the tests fail if I have a ~/.xrayconfig file present. That's because the default editor is only used when ~/.xrayconfig is absent.

I think we'll have to refactor a bit further to make the tests reliable. Any ideas?

[dunn]

Sorry I let this sit; I'll try to remember this weekend to look into the .xrayconfig issue.

[dunn]

Done; sorry for the delay!

[mattbrictson]

Great! Thanks for a great PR!

[mattbrictson]

I'll publish a new release by the end of the week.

[mattbrictson]

v0.1.22 has been released!