Skip to content

fix: handling of EDITOR env var #10855

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

Merged
merged 1 commit into from
Jul 8, 2025
Merged

fix: handling of EDITOR env var #10855

merged 1 commit into from
Jul 8, 2025

Conversation

gammazero
Copy link
Contributor

The ipfs config edit command did not correctly handle the EDITOR environment variable correctly when its value contains flags and arguments, i.e. EDITOR=emacs -nw. The command was treating the entire value of $EDITOR as the name of the editor command. This has been fixed to parse the value of $EDITOR into separate args, respecting shell quoting.

Closes #9375

The `ipfs config edit` command did not correctly handle the `EDITOR` environment variable correctly when its value contains flags and arguments, i.e. `EDITOR=emacs -nw`. The command was treating the entire value of `$EDITOR` as the name of the editor command. This has been fixed to parse the value of `$EDITOR` into separate args, respecting shell quoting.

Closes #9375
@gammazero gammazero requested a review from a team as a code owner July 3, 2025 04:52
@@ -512,7 +512,14 @@ func editConfig(filename string) error {
return errors.New("ENV variable $EDITOR not set")
}

cmd := exec.Command(editor, filename)
editorAndArgs, err := shlex.Split(editor, true)
Copy link
Member

Choose a reason for hiding this comment

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

Note: go-shlex has not had a commit in 5 years. However, the code of go-shlex is 193 lines long with some sane tests. Perhaps no changes are needed because it works perfectly?

I didn't see any Windows-specific tests.

Copy link
Contributor Author

@gammazero gammazero Jul 8, 2025

Choose a reason for hiding this comment

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

Note that google/shlex does not handle POSIX correctly in the case of the last argument in this example:
https://go.dev/play/p/EuYSLJB8p5t

Maybe that does not matter?

Copy link
Member

Choose a reason for hiding this comment

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

i think using anmitsu/go-shlex is fine, we have it pulled by quic-go's deps anyway

@gammazero gammazero merged commit 4195a1d into master Jul 8, 2025
16 checks passed
@gammazero gammazero deleted the fix-editor-env-handling branch July 8, 2025 16:00
@lidel lidel mentioned this pull request Jul 8, 2025
46 tasks
lidel pushed a commit that referenced this pull request Jul 8, 2025
The `ipfs config edit` command did not correctly handle the `EDITOR` environment variable correctly when its value contains flags and arguments, i.e. `EDITOR=emacs -nw`. The command was treating the entire value of `$EDITOR` as the name of the editor command. This has been fixed to parse the value of `$EDITOR` into separate args, respecting shell quoting.

Closes #9375

(cherry picked from commit 4195a1d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ipfs config edit doesn't handle EDITOR env var properly
3 participants