Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Error when using EDITOR environment variable with arguments #44

Merged
merged 2 commits into from
Mar 6, 2022

Conversation

stefan-fast
Copy link
Contributor

Hello there!
First of all, thanks for this awesome little project :)

Description

This PR tries to fix the issue of using the tt edit command in combination with an EDITOR environment variable that consists of a command with arguments.

Issue

Example

I am using EDITOR="code --wait" as my environment variable for starting Visual Studio Code. Currently, this gives the following output:

export EDITOR="code --wait"tt edit
Error: exec: "code --wait": executable file not found in $PATH

Reason

The whole string "code --wait" is passed to exec.Command as the first argument. By documentation, only the command itself should be the first argument of exec.Command and any arguments of that command should follow as seperate arguments.

Solution

If there are any spaces in the environment variable EDITOR, the command and it's argument are split into two seperate arguments for exec.Command.

In my testing both

export EDITOR="code --wait"tt edit

and

export EDITOR="nano"tt edit

worked fine with the proposed fix.

@stefan-fast stefan-fast changed the title Fix: Error when using EDITOR environment variable with arguments fix: Error when using EDITOR environment variable with arguments Mar 4, 2022
Comment on lines 38 to 46

editorArgs := []string{tmp}
if strings.ContainsAny(editor, " ") {
editorParts := strings.Split(editor, " ")
editor = editorParts[0]
editorArgs = append(editorArgs, editorParts[1:]...)
}

edit := exec.Command(editor, editorArgs...)
Copy link
Owner

@caarlos0 caarlos0 Mar 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can make this a bit nicer:

editor := strings.Fields(os.Getenv("EDITOR"))
if len(editor) == 0 {
  return fmt.Errorf("no $EDITOR set")
}

cmd := editor[0]
var args []string
if len(editor) > 1 {
  args = append(args, editor[1:])
}
args = append(args, tmp)

edit := exec.Command(cmd, args)
// ...

wdyt?

Copy link
Contributor Author

@stefan-fast stefan-fast Mar 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I didn't know strings.Fields yet. Thanks for the suggestion, I'll update the PR in just a moment. I have to rename the variables though since cmd and args are the parameter names of the function already :)

@caarlos0 caarlos0 merged commit 0b127cd into caarlos0:main Mar 6, 2022
@caarlos0
Copy link
Owner

caarlos0 commented Mar 6, 2022

thanks!

@caarlos0 caarlos0 added the enhancement New feature or request label Mar 6, 2022
@stefan-fast stefan-fast deleted the fix/editor-env-with-arguments branch March 8, 2022 23:32
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request size/S
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants