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

git: fix edit command when using extra args #748

Merged
merged 1 commit into from
Oct 6, 2021

Conversation

bmeneg
Copy link
Collaborator

@bmeneg bmeneg commented Oct 5, 2021

The user can include extra arguments to the git core.editor configuration
option or the GIT_EDITOR env var, however, in the current way that
editorCMD() is coded, all the extra arguments are passed to the actual
editor executable as a single argument: argv[0] == exec, argv[1] == all
arguments, instead of splitting them in separated argv[] fields. Although
some editors are fine with this behavior, because they split the argv array
before actually using it, some others don't.

Since splitting all the args, one per argv field, is the standard across the
industry, we should do the same. This patch refactors the editorCMD() code
to ease its understanding, split each arg in one argv field, and also to
solve an issue, found during the refactoring, for when the user hasn't set
neither git core.editor nor GIT_EDITOR.

Signed-off-by: Bruno Meneguele bmeneg@redhat.com

Fixes: #747

@bmeneg bmeneg requested review from prarit and zampierilucas October 5, 2021 18:57
The user can include extra arguments to the git core.editor configuration
option or the GIT_EDITOR env var, however, in the current way that
editorCMD() is coded, all the extra arguments are passed to the actual
editor executable as a single argument: argv[0] == exec, argv[1] == all
arguments, instead of splitting them in separated argv[] fields. Although
some editors are fine with this behavior, because they split the argv array
before actually using it, some others don't.

Since splitting all the args, one per argv field, is the standard across the
industry, we should do the same. This patch refactors the editorCMD() code
to ease its understanding, split each arg in one argv field, and also to
solve an issue, found during the refactoring, for when the user hasn't set
neither git core.editor nor GIT_EDITOR.

Signed-off-by: Bruno Meneguele <bmeneg@redhat.com>
@codecov
Copy link

codecov bot commented Oct 5, 2021

Codecov Report

Merging #748 (b73579a) into master (6747135) will increase coverage by 0.03%.
The diff coverage is 80.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #748      +/-   ##
==========================================
+ Coverage   54.76%   54.79%   +0.03%     
==========================================
  Files          77       77              
  Lines        5606     5608       +2     
==========================================
+ Hits         3070     3073       +3     
  Misses       2255     2255              
+ Partials      281      280       -1     
Impacted Files Coverage Δ
internal/git/edit.go 65.95% <80.00%> (+1.82%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6747135...b73579a. Read the comment docs.

@bmeneg bmeneg merged commit 7540b64 into zaquestion:master Oct 6, 2021
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.

lab mr create does not work when the editor takes arguments
2 participants