When run lucky exec, get vim: command not found error.

[zw963]

Describe the bug

 ╰─ $ 1  lucky exec
--: line 1: vim: command not found
Compiling Code...

ENTER to edit, q to quit   # <= pressing Enter here, then ...

cp: './tmp/console/1654853646934_console.cr' and './tmp/console/1654853646934_console.cr' are the same file
--: line 1: vim: command not found
Compiling Code...
ENTER to edit, q to quit

Expected behavior It should respect my $EDITOR environment variable. as almost most of tools which depend on this.

e.g. git, original cry

For now, i have to install vim to make lucky exec work. but, i use prefer to use emacs.

Versions (please complete the following information):

• Lucky version (check in shard.lock): 0.30.1
• Crystal version (crystal --version):

  
  Crystal 1.4.1 [b7377c041] (2022-04-22)

LLVM: 10.0.0 Default target: x86_64-unknown-linux-gnu

 - OS: Arch Linux.

[zw963]

This is consider as a bug, i thought i can try fix this with a PR.

[jwoertink]

Hey, thanks for reporting. I don't think we account for $EDITOR anywhere. To change the editor you need to --editor=xyz flag https://github.com/luckyframework/lucky/blob/88f9562a5d69dd9c90af03967757daeed376af17/tasks/exec.cr#L8

We actually don't use the original Cry version, we have a forked version here: https://github.com/luckyframework/cry

If you wanted to submit a PR, this should be easy to change...

https://github.com/luckyframework/lucky/blob/88f9562a5d69dd9c90af03967757daeed376af17/tasks/exec.cr#L36

I think this just needs to be ENV["EDITOR"]? || editor || settings.editor

[zw963]

I think this just needs to be ENV["EDITOR"]? || editor || settings.editor

No, we should respect user provided editor with "-e", i consider we should add it as a default value instead at https://github.com/luckyframework/lucky/blob/88f9562a5d69dd9c90af03967757daeed376af17/tasks/exec.cr#L16

I don't think we account for $EDITOR anywhere.

But, if i am not misunderstood, you means, lucky never accept this EDITOR PR, right? i consider force all user to use Vim is not a good idea.

[jwoertink]

You're not forced to use Vim, that's just the default. Right now there's 2 options:

1. lucky exec -e emacs
2. Setup a config file in config/editor.cr

   # config/editor.cr
   Lucky::Exec.configure do |setting|
   setting.editor = ENV["EDITOR"]? || "emacs"
   end

The Lucky code doesn't use $EDITOR anyway currently, but I'm ok with adding it. I don't think it should be default though. Or, if we do make it default, we need a fallback. setting editor : String = ENV["EDITOR"]? || "vim"

How about we do editor_to_use = editor || ENV["EDITOR"]? || settings.editor? Then the flag comes first, then ENV, then the default if nothing else is specified.

[zw963]

Or, if we do make it default, we need a fallback. setting editor : String = ENV["EDITOR"]? || "vim"

Yes， this exactly what i means.

How about we do editor_to_use = editor || ENV["EDITOR"]? || settings.editor? Then the flag comes first, then ENV, then the default if nothing else is specified.

I consider this same as setting editor : String = ENV["EDITOR"]? || "vim". :smile: