* [PATCH] sequencer: allow disabling conflict advice
From: Philippe Blain via GitGitGadget @ 2024-03-02 16:18 UTC (permalink / raw)
To: git; +Cc: Philippe Blain, Philippe Blain
From: Philippe Blain <levraiphilippeblain@gmail.com>
Allow disabling the advice shown when a squencer operation results in a
merge conflict through a new config 'advice.sequencerConflict'.
Update the tests accordingly. Note that the body of the second test in
t3507-cherry-pick-conflict.sh is enclosed in double quotes, so we must
escape them in the added line.
Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
---
sequencer: allow disabling conflict advice
CC: Elijah Newren newren@gmail.com CC: Phillip Wood
phillip.wood@dunelm.org.uk CC: Johannes Schindelin
Johannes.Schindelin@gmx.de CC: ZheNing Hu adlternative@gmail.com
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1682%2Fphil-blain%2Fsequencer-conflict-advice-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1682/phil-blain/sequencer-conflict-advice-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1682
Documentation/config/advice.txt | 3 +++
advice.c | 1 +
advice.h | 1 +
sequencer.c | 33 ++++++++++++++++++---------------
t/t3501-revert-cherry-pick.sh | 1 +
t/t3507-cherry-pick-conflict.sh | 2 ++
6 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt
index c7ea70f2e2e..736b88407a4 100644
--- a/Documentation/config/advice.txt
+++ b/Documentation/config/advice.txt
@@ -104,6 +104,9 @@ advice.*::
rmHints::
In case of failure in the output of linkgit:git-rm[1],
show directions on how to proceed from the current state.
+ sequencerConflict::
+ Advice shown when a sequencer operation stops because
+ of conflicts.
sequencerInUse::
Advice shown when a sequencer command is already in progress.
skippedCherryPicks::
diff --git a/advice.c b/advice.c
index 6e9098ff089..23e48194e74 100644
--- a/advice.c
+++ b/advice.c
@@ -71,6 +71,7 @@ static struct {
[ADVICE_RESET_NO_REFRESH_WARNING] = { "resetNoRefresh" },
[ADVICE_RESOLVE_CONFLICT] = { "resolveConflict" },
[ADVICE_RM_HINTS] = { "rmHints" },
+ [ADVICE_SEQUENCER_CONFLICT] = { "sequencerConflict" },
[ADVICE_SEQUENCER_IN_USE] = { "sequencerInUse" },
[ADVICE_SET_UPSTREAM_FAILURE] = { "setUpstreamFailure" },
[ADVICE_SKIPPED_CHERRY_PICKS] = { "skippedCherryPicks" },
diff --git a/advice.h b/advice.h
index 9d4f49ae38b..98966f8991d 100644
--- a/advice.h
+++ b/advice.h
@@ -40,6 +40,7 @@ enum advice_type {
ADVICE_RESOLVE_CONFLICT,
ADVICE_RM_HINTS,
ADVICE_SEQUENCER_IN_USE,
+ ADVICE_SEQUENCER_CONFLICT,
ADVICE_SET_UPSTREAM_FAILURE,
ADVICE_SKIPPED_CHERRY_PICKS,
ADVICE_STATUS_AHEAD_BEHIND_WARNING,
diff --git a/sequencer.c b/sequencer.c
index f49a871ac06..3e2f028ce2d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -467,7 +467,7 @@ static void print_advice(struct repository *r, int show_hint,
char *msg = getenv("GIT_CHERRY_PICK_HELP");
if (msg) {
- advise("%s\n", msg);
+ advise_if_enabled(ADVICE_SEQUENCER_CONFLICT, "%s\n", msg);
/*
* A conflict has occurred but the porcelain
* (typically rebase --interactive) wants to take care
@@ -480,22 +480,25 @@ static void print_advice(struct repository *r, int show_hint,
if (show_hint) {
if (opts->no_commit)
- advise(_("after resolving the conflicts, mark the corrected paths\n"
- "with 'git add <paths>' or 'git rm <paths>'"));
+ advise_if_enabled(ADVICE_SEQUENCER_CONFLICT,
+ _("after resolving the conflicts, mark the corrected paths\n"
+ "with 'git add <paths>' or 'git rm <paths>'"));
else if (opts->action == REPLAY_PICK)
- advise(_("After resolving the conflicts, mark them with\n"
- "\"git add/rm <pathspec>\", then run\n"
- "\"git cherry-pick --continue\".\n"
- "You can instead skip this commit with \"git cherry-pick --skip\".\n"
- "To abort and get back to the state before \"git cherry-pick\",\n"
- "run \"git cherry-pick --abort\"."));
+ advise_if_enabled(ADVICE_SEQUENCER_CONFLICT,
+ _("After resolving the conflicts, mark them with\n"
+ "\"git add/rm <pathspec>\", then run\n"
+ "\"git cherry-pick --continue\".\n"
+ "You can instead skip this commit with \"git cherry-pick --skip\".\n"
+ "To abort and get back to the state before \"git cherry-pick\",\n"
+ "run \"git cherry-pick --abort\"."));
else if (opts->action == REPLAY_REVERT)
- advise(_("After resolving the conflicts, mark them with\n"
- "\"git add/rm <pathspec>\", then run\n"
- "\"git revert --continue\".\n"
- "You can instead skip this commit with \"git revert --skip\".\n"
- "To abort and get back to the state before \"git revert\",\n"
- "run \"git revert --abort\"."));
+ advise_if_enabled(ADVICE_SEQUENCER_CONFLICT,
+ _("After resolving the conflicts, mark them with\n"
+ "\"git add/rm <pathspec>\", then run\n"
+ "\"git revert --continue\".\n"
+ "You can instead skip this commit with \"git revert --skip\".\n"
+ "To abort and get back to the state before \"git revert\",\n"
+ "run \"git revert --abort\"."));
else
BUG("unexpected pick action in print_advice()");
}
diff --git a/t/t3501-revert-cherry-pick.sh b/t/t3501-revert-cherry-pick.sh
index aeab689a98d..bc7c878b236 100755
--- a/t/t3501-revert-cherry-pick.sh
+++ b/t/t3501-revert-cherry-pick.sh
@@ -170,6 +170,7 @@ test_expect_success 'advice from failed revert' '
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".
+ hint: Disable this message with "git config advice.sequencerConflict false"
EOF
test_commit --append --no-tag "double-add dream" dream dream &&
test_must_fail git revert HEAD^ 2>actual &&
diff --git a/t/t3507-cherry-pick-conflict.sh b/t/t3507-cherry-pick-conflict.sh
index c88d597b126..a643893dcbd 100755
--- a/t/t3507-cherry-pick-conflict.sh
+++ b/t/t3507-cherry-pick-conflict.sh
@@ -60,6 +60,7 @@ test_expect_success 'advice from failed cherry-pick' '
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
+ hint: Disable this message with "git config advice.sequencerConflict false"
EOF
test_must_fail git cherry-pick picked 2>actual &&
@@ -74,6 +75,7 @@ test_expect_success 'advice from failed cherry-pick --no-commit' "
error: could not apply \$picked... picked
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add <paths>' or 'git rm <paths>'
+ hint: Disable this message with \"git config advice.sequencerConflict false\"
EOF
test_must_fail git cherry-pick --no-commit picked 2>actual &&
base-commit: 0f9d4d28b7e6021b7e6db192b7bf47bd3a0d0d1d
--
gitgitgadget
^ permalink raw reply related
* [PATCH v2 5/5] completion: reflog subcommands and options
From: Rubén Justo @ 2024-03-02 15:52 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
In-Reply-To: <ea6c8890-9ff3-46c9-b933-6a52083b1001@gmail.com>
Make generic the completion for reflog subcommands and its options.
Note that we still need to special case the options for "show".
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
contrib/completion/git-completion.bash | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f9fbf1f703..c5c9e9de2d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2445,8 +2445,12 @@ _git_rebase ()
_git_reflog ()
{
- local subcommands="show delete expire"
- local subcommand="$(__git_find_subcommand "$subcommands" "show")"
+ local subcommands subcommand
+
+ __git_resolve_builtins "reflog"
+
+ subcommands="$___git_resolved_builtins"
+ subcommand="$(__git_find_subcommand "$subcommands" "show")"
case "$subcommand,$cur" in
show,--*)
@@ -2455,6 +2459,10 @@ _git_reflog ()
"
return
;;
+ $subcommand,--*)
+ __gitcomp_builtin "reflog_$subcommand"
+ return
+ ;;
esac
__git_complete_refs
--
2.44.0
^ permalink raw reply related
* [PATCH v2 4/5] completion: factor out __git_resolve_builtins
From: Rubén Justo @ 2024-03-02 15:52 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
In-Reply-To: <ea6c8890-9ff3-46c9-b933-6a52083b1001@gmail.com>
We're going to use the result of "git xxx --git-completion-helper" not
only for feeding COMPREPLY.
Therefore, factor out the execution and the caching of its results in
__gitcomp_builtin, to a new function __git_resolve_builtins.
While we're here, move an important comment we have in the function to
its header, so it gains visibility.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
I've changed my mind last minute, and renamed the function to
__git_resolve_builtins, so the subject of this patch is different
from the one included in the cover letter.
contrib/completion/git-completion.bash | 31 +++++++++++++++++++++-----
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dc5f73a9f3..f9fbf1f703 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -452,16 +452,18 @@ fi
# This function is equivalent to
#
-# __gitcomp "$(git xxx --git-completion-helper) ..."
+# ___git_resolved_builtins=$(git xxx --git-completion-helper)
#
-# except that the output is cached. Accept 1-3 arguments:
+# except that the result of the execution is cached.
+#
+# Accept 1-3 arguments:
# 1: the git command to execute, this is also the cache key
+# (use "_" when the command contains spaces, e.g. "remote add"
+# becomes "remote_add")
# 2: extra options to be added on top (e.g. negative forms)
# 3: options to be excluded
-__gitcomp_builtin ()
+__git_resolve_builtins ()
{
- # spaces must be replaced with underscore for multi-word
- # commands, e.g. "git remote add" becomes remote_add.
local cmd="$1"
local incl="${2-}"
local excl="${3-}"
@@ -487,7 +489,24 @@ __gitcomp_builtin ()
eval "$var=\"$options\""
fi
- __gitcomp "$options"
+ ___git_resolved_builtins="$options"
+}
+
+# This function is equivalent to
+#
+# __gitcomp "$(git xxx --git-completion-helper) ..."
+#
+# except that the output is cached. Accept 1-3 arguments:
+# 1: the git command to execute, this is also the cache key
+# (use "_" when the command contains spaces, e.g. "remote add"
+# becomes "remote_add")
+# 2: extra options to be added on top (e.g. negative forms)
+# 3: options to be excluded
+__gitcomp_builtin ()
+{
+ __git_resolve_builtins "$1" "$2" "$3"
+
+ __gitcomp "$___git_resolved_builtins"
}
# Variation of __gitcomp_nl () that appends to the existing list of
--
2.44.0
^ permalink raw reply related
* [PATCH v2 2/5] completion: introduce __git_find_subcommand
From: Rubén Justo @ 2024-03-02 15:51 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
In-Reply-To: <ea6c8890-9ff3-46c9-b933-6a52083b1001@gmail.com>
Let's have a function to get the current subcommand when completing
commands that follow the syntax:
git <command> <subcommand>
As a convenience, let's allow an optional "default subcommand" to be
returned if none is found.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
contrib/completion/git-completion.bash | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ff216f1c65..849d191b02 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -554,6 +554,26 @@ __gitcomp_file ()
true
}
+# Find the current subcommand for commands that follow the syntax:
+#
+# git <command> <subcommand>
+#
+# 1: List of possible subcommands.
+# 2: Optional subcommand to return when none is found.
+__git_find_subcommand ()
+{
+ local subcommand subcommands="$1" default_subcommand="$2"
+
+ for subcommand in $subcommands; do
+ if [ "$subcommand" = "${words[__git_cmd_idx+1]}" ]; then
+ echo $subcommand
+ return
+ fi
+ done
+
+ echo $default_subcommand
+}
+
# Execute 'git ls-files', unless the --committable option is specified, in
# which case it runs 'git diff-index' to find out the files that can be
# committed. It return paths relative to the directory specified in the first
--
2.44.0
^ permalink raw reply related
* [PATCH v2 3/5] completion: reflog show <log-options>
From: Rubén Justo @ 2024-03-02 15:50 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
In-Reply-To: <ea6c8890-9ff3-46c9-b933-6a52083b1001@gmail.com>
Let's add completion for <log-options> in "reflog show" so that the user
can easily discover uses like:
$ git reflog --since=1.day.ago
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
contrib/completion/git-completion.bash | 10 ++++++++++
t/t9902-completion.sh | 5 ++++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 849d191b02..dc5f73a9f3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2427,6 +2427,16 @@ _git_rebase ()
_git_reflog ()
{
local subcommands="show delete expire"
+ local subcommand="$(__git_find_subcommand "$subcommands" "show")"
+
+ case "$subcommand,$cur" in
+ show,--*)
+ __gitcomp "
+ $__git_log_common_options
+ "
+ return
+ ;;
+ esac
__git_complete_refs
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index dbd57e6a28..04f3620e5b 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2626,7 +2626,10 @@ test_expect_success 'git reflog show' '
shown Z
EOF
test_completion "git reflog show sho" "shown " &&
- test_completion "git reflog shown sho" "shown "
+ test_completion "git reflog shown sho" "shown " &&
+ test_completion "git reflog --unt" "--until=" &&
+ test_completion "git reflog show --unt" "--until=" &&
+ test_completion "git reflog shown --unt" "--until="
'
test_expect_success 'options with value' '
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v6 0/2] Implement `git log --merge` also for rebase/cherry-pick/revert
From: Philippe Blain @ 2024-03-02 15:35 UTC (permalink / raw)
To: phillip.wood, git
Cc: Johannes Sixt, Elijah Newren, Michael Lohmann, Patrick Steinhardt,
Junio C Hamano, Michael Lohmann
In-Reply-To: <3875a0dc-6ef2-4bd3-9852-c180043464c3@gmail.com>
Hi Phillip,
Le 2024-02-28 à 09:40, phillip.wood123@gmail.com a écrit :
> Hi Philippe
>
> On 28/02/2024 13:54, Philippe Blain wrote:
>> Range-diff versus v5:
>>
>> 1: c9536431d1 ! 1: 363657561c revision: ensure MERGE_HEAD is a ref in prepare_show_merge
>> @@ Commit message
>> (2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()
>> (3) error out when MERGE_HEAD is a symref.
>> + Note that we avoid marking the new error message for translation as it
>> + will be done in the next commit when the message is generalized to other
>> + special refs.
>
> Looking at the change below, the new message is in fact marked for translation. I don't think this matters (other than the commit message being confusing) as the translators will only see the final version of the massage.
Oops, you are right, I wanted to do what I wrote but ended up marking it also in 1/2.
Junio already merged it to next, so I guess it's gonna stay that way.
Thanks for your review,
Philippe.
^ permalink raw reply
* [PATCH v2 1/5] completion: reflog with implicit "show"
From: Rubén Justo @ 2024-03-02 14:37 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
In-Reply-To: <ea6c8890-9ff3-46c9-b933-6a52083b1001@gmail.com>
When no subcommand is specified to "reflog", we assume "show" [1]:
$ git reflog -h
usage: git reflog [show] [<log-options>] [<ref>]
...
This implicit "show" is not being completed correctly:
$ git checkout -b default
$ git reflog def<TAB><TAB>
... no completion options ...
The expected result is:
$ git reflog default
This happens because we're completing references after seeing a valid
subcommand in the command line. This prevents the implicit "show" from
working properly, but also introduces a new problem: it keeps offering
subcommand options when the subcommand is implicit:
$ git checkout -b explore
$ git reflog default ex<TAB>
...
$ git reflog default expire
The expected result is:
$ git reflog default explore
To fix this, complete references even if no subcommand is present, or in
other words when the subcommand is implicit "show".
Also, only include completion options for subcommands when completing
the right position in the command line.
1. cf39f54efc (git reflog show, 2007-02-08)
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
contrib/completion/git-completion.bash | 9 ++++-----
t/t9902-completion.sh | 11 +++++++++++
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8c40ade494..ff216f1c65 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2407,12 +2407,11 @@ _git_rebase ()
_git_reflog ()
{
local subcommands="show delete expire"
- local subcommand="$(__git_find_on_cmdline "$subcommands")"
- if [ -z "$subcommand" ]; then
- __gitcomp "$subcommands"
- else
- __git_complete_refs
+ __git_complete_refs
+
+ if [ $((cword - __git_cmd_idx)) -eq 1 ]; then
+ __gitcompappend "$subcommands" "" "$cur" " "
fi
}
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index aa9a614de3..dbd57e6a28 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -2618,6 +2618,17 @@ test_expect_success 'git clone --config= - value' '
EOF
'
+test_expect_success 'git reflog show' '
+ test_when_finished "git checkout - && git branch -d shown" &&
+ git checkout -b shown &&
+ test_completion "git reflog sho" <<-\EOF &&
+ show Z
+ shown Z
+ EOF
+ test_completion "git reflog show sho" "shown " &&
+ test_completion "git reflog shown sho" "shown "
+'
+
test_expect_success 'options with value' '
test_completion "git merge -X diff-algorithm=" <<-\EOF
--
2.44.0
^ permalink raw reply related
* [PATCH v2 0/5] completion for git-reflog show
From: Rubén Justo @ 2024-03-02 14:30 UTC (permalink / raw)
To: Git List; +Cc: Junio C Hamano
In-Reply-To: <98daf977-dbad-4d3b-a293-6a769895088f@gmail.com>
This iteration has three main changes and two new commits since v2.
The changes:
- The function __gitcomp_subcommand is no longer needed in this series.
The patch that introduced it has been removed.
- The description for the behaviour we're fixing in the patch 1/5 has
been reworded to better explain the behaviour we're expecting.
- Provide completion for both subcommands and references when the
expectation is such that we're not sure if the implicit subcommand
"show" is desired.
About the two new commits:
This series is described in the last "What's cooking" with:
The command line completion script (in contrib/) learned to
complete "git reflog" better.
So, while we're here I've included two new commits, 4/5 and 5/5, that
fit well in that description.
An important note is in the last patch, reporting the special case
needed for the <log-options> in the "show" subcommand. This is what has
made me decide to include the new commits in this series.
Rubén Justo (5):
completion: reflog with implicit "show"
completion: introduce __git_find_subcommand
completion: reflog show <log-options>
completion: factor out __git_builtin
completion: reflog subcommands and options
contrib/completion/git-completion.bash | 70 ++++++++++++++++++++++----
t/t9902-completion.sh | 14 ++++++
2 files changed, 73 insertions(+), 11 deletions(-)
Range-diff against v1:
1: 1a76491362 < -: ---------- completion: introduce __gitcomp_subcommand
3: 8defb041ac ! 1: 61b9696238 completion: reflog with implicit "show"
@@ Commit message
usage: git reflog [show] [<log-options>] [<ref>]
...
- We are not completing correctly this implicit uses of "show":
-
- With ...
+ This implicit "show" is not being completed correctly:
$ git checkout -b default
+ $ git reflog def<TAB><TAB>
+ ... no completion options ...
- ... we are not completing "default":
+ The expected result is:
- $ git reflog def<TAB><TAB>
+ $ git reflog default
+
+ This happens because we're completing references after seeing a valid
+ subcommand in the command line. This prevents the implicit "show" from
+ working properly, but also introduces a new problem: it keeps offering
+ subcommand options when the subcommand is implicit:
+
+ $ git checkout -b explore
+ $ git reflog default ex<TAB>
+ ...
+ $ git reflog default expire
- And we are incorrectly returning the "subcommands" when:
+ The expected result is:
- $ git reflog default <TAB><TAB>
- delete expire show
+ $ git reflog default explore
- Let's use __gitcomp_subcommand to correctly handle implicit
- subcommands.
+ To fix this, complete references even if no subcommand is present, or in
+ other words when the subcommand is implicit "show".
+
+ Also, only include completion options for subcommands when completing
+ the right position in the command line.
1. cf39f54efc (git reflog show, 2007-02-08)
@@ contrib/completion/git-completion.bash: _git_rebase ()
- __gitcomp "$subcommands"
- else
- __git_complete_refs
-+ if __gitcomp_subcommand "$subcommands"; then
-+ return
- fi
-+
+ __git_complete_refs
++
++ if [ $((cword - __git_cmd_idx)) -eq 1 ]; then
++ __gitcompappend "$subcommands" "" "$cur" " "
+ fi
}
- __git_send_email_confirm_options="always never auto cc compose"
## t/t9902-completion.sh ##
@@ t/t9902-completion.sh: test_expect_success 'git clone --config= - value' '
@@ t/t9902-completion.sh: test_expect_success 'git clone --config= - value' '
'
+test_expect_success 'git reflog show' '
-+ test_when_finished "git checkout -" &&
++ test_when_finished "git checkout - && git branch -d shown" &&
+ git checkout -b shown &&
-+ test_completion "git reflog sho" "show " &&
++ test_completion "git reflog sho" <<-\EOF &&
++ show Z
++ shown Z
++ EOF
+ test_completion "git reflog show sho" "shown " &&
+ test_completion "git reflog shown sho" "shown "
+'
2: b1aad9a667 ! 2: b3133c69d3 completion: introduce __git_find_subcommand
@@ Commit message
Signed-off-by: Rubén Justo <rjusto@gmail.com>
## contrib/completion/git-completion.bash ##
-@@ contrib/completion/git-completion.bash: __gitcomp_subcommand ()
- fi
+@@ contrib/completion/git-completion.bash: __gitcomp_file ()
+ true
}
+# Find the current subcommand for commands that follow the syntax:
4: 4d3fb1d563 ! 3: e6e526b436 completion: reflog show <log-options>
@@ Commit message
## contrib/completion/git-completion.bash ##
@@ contrib/completion/git-completion.bash: _git_rebase ()
-
_git_reflog ()
{
-- local subcommands="show delete expire"
-+ local subcommand subcommands="show delete expire"
-
- if __gitcomp_subcommand "$subcommands"; then
- return
- fi
-
-+ subcommand="$(__git_find_subcommand "$subcommands" "show")"
+ local subcommands="show delete expire"
++ local subcommand="$(__git_find_subcommand "$subcommands" "show")"
+
+ case "$subcommand,$cur" in
+ show,--*)
@@ contrib/completion/git-completion.bash: _git_rebase ()
+ return
+ ;;
+ esac
-+
+
__git_complete_refs
- }
## t/t9902-completion.sh ##
@@ t/t9902-completion.sh: test_expect_success 'git reflog show' '
- git checkout -b shown &&
- test_completion "git reflog sho" "show " &&
+ shown Z
+ EOF
test_completion "git reflog show sho" "shown " &&
- test_completion "git reflog shown sho" "shown "
+ test_completion "git reflog shown sho" "shown " &&
-: ---------- > 4: dfed95d495 completion: factor out __git_builtin
-: ---------- > 5: 8193b7f4f9 completion: reflog subcommands and options
--
2.44.0
^ permalink raw reply
* Re: [GSOC][PATCH] userdiff: Add JavaScript function patterns
From: Christian Couder @ 2024-03-02 10:54 UTC (permalink / raw)
To: Sergius Nyah; +Cc: git, gitster, pk, shyamthakkar001
In-Reply-To: <CAP8UFD1sAYORc_q20t4Y28mSjNa0BZ69XQ-hGFsNTzbThsG1KQ@mail.gmail.com>
On Sat, Mar 2, 2024 at 11:28 AM Christian Couder
<christian.couder@gmail.com> wrote:
>
> The subject should be "userdiff: add JavaScript function patterns" so
> with "add" instead of "Add". See the SubmittingPatches document which
> contains:
>
> "The title sentence after the "area:" prefix omits the full stop at the
> end, and its first word is not capitalized (the omission
> of capitalization applies only to the word after the "area:"
> prefix of the title) unless there is a reason to
> capitalize it other than because it is the first word in the sentence."
Also about the subject, please use v2, v3, v4 when you resend a patch
or a patch series that you have already sent. `git format-patch` has a
"-v <n>" or "--reroll-count=<n>" option for that purpose.
^ permalink raw reply
* Re: [GSOC][PATCH] userdiff: Add JavaScript function patterns
From: Christian Couder @ 2024-03-02 10:28 UTC (permalink / raw)
To: Sergius Nyah; +Cc: git, gitster, pk, shyamthakkar001
In-Reply-To: <20240301074048.188835-1-sergiusnyah@gmail.com>
The subject should be "userdiff: add JavaScript function patterns" so
with "add" instead of "Add". See the SubmittingPatches document which
contains:
"The title sentence after the "area:" prefix omits the full stop at the
end, and its first word is not capitalized (the omission
of capitalization applies only to the word after the "area:"
prefix of the title) unless there is a reason to
capitalize it other than because it is the first word in the sentence."
On Fri, Mar 1, 2024 at 8:40 AM Sergius Nyah <sergiusnyah@gmail.com> wrote:
>
> This commit adds a patterns used to match JavaScript functions.
It should be either "add patterns" or "add a pattern".
Also instead of "This commit" please use the imperative mood or
"Let's". See the SubmittingPatches document which contains:
"Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behavior."
> It now correctly identifies function declarations, function expressions,
> and functions defined inside blocks. Add test for corresponding change in userdiff.
>
> Signed-off-by: Sergius Nyah <sergiusnyah@gmail.com>
> ---
> t/t4018-diff-funcname.sh | 22 ++++++++++++++++++++++
> userdiff.c | 12 ++++++++++++
> 2 files changed, 34 insertions(+)
In t4034-diff-words.sh there is:
test_language_driver ada
test_language_driver bibtex
test_language_driver cpp
test_language_driver csharp
test_language_driver css
test_language_driver dts
test_language_driver fortran
test_language_driver html
test_language_driver java
test_language_driver kotlin
test_language_driver matlab
test_language_driver objc
test_language_driver pascal
test_language_driver perl
test_language_driver php
test_language_driver python
test_language_driver ruby
test_language_driver scheme
test_language_driver tex
So I would have thought that you would just add a
`test_language_driver javascript` line in this file and associated
material in the t/t4034/ directory.
> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
> index e026fac1f4..d35cce18a0 100755
> --- a/t/t4018-diff-funcname.sh
> +++ b/t/t4018-diff-funcname.sh
> @@ -120,3 +120,25 @@ do
> done
>
> test_done
> +
> +test_expect_success 'identify builtin patterns in JavaScript' '
> + # setup
> + echo "function myFunction() { return true; }" > test.js &&
> + echo "var myVar = function() { return false; }" >> test.js &&
> + git add test.js &&
> + git commit -m "add test.js" &&
> +
> + # modify the file
> + echo "function myFunction() { return false; }" > test.js &&
> + echo "var myVar = function() { return true; }" >> test.js &&
> +
> + # command under test
> + git diff >output &&
> +
> + # check results
> + test_i18ngrep "function myFunction() { return true; }" output &&
> + test_i18ngrep "function myFunction() { return false; }" output &&
> + test_i18ngrep "var myVar = function() { return false; }" output &&
> + test_i18ngrep "var myVar = function() { return true; }" output
I think we try to use just test_grep instead of test_i18ngrep these days.
> +'
> +test_done
> \ No newline at end of file
Please add a new line at the end of this file if you still change it.
Thanks!
^ permalink raw reply
* Re: [PATCH 2/2] diff: add 'diff.wordDiff' config option
From: Eric Sunshine @ 2024-03-02 10:25 UTC (permalink / raw)
To: Karthik Nayak; +Cc: oliver, git
In-Reply-To: <20240302095751.123138-3-karthik.188@gmail.com>
On Sat, Mar 2, 2024 at 4:58 AM Karthik Nayak <karthik.188@gmail.com> wrote:
> The git-diff(1) command supports the `--word-diff` which allows the
> users to specify how to delimit word diffs. Provide this option also as
> a config param 'diff.wordDiff'.
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
> ---
> diff --git a/diff.c b/diff.c
> @@ -209,6 +210,23 @@ int git_config_rename(const char *var, const char *value)
> +static int parse_word_diff_value(const char *value)
> +{
> + if (!value)
> + return -1;
> + else if (!strcmp(value, "plain"))
> + return DIFF_WORDS_PLAIN;
> + else if (!strcmp(value, "color")) {
> + return DIFF_WORDS_COLOR;
> + }
> + else if (!strcmp(value, "porcelain"))
> + return DIFF_WORDS_PORCELAIN;
> + else if (!strcmp(value, "none"))
> + return DIFF_WORDS_NONE;
> +
> + return -1;
> +}
Why is one arm surrounded by curly braces when none of the others are?
^ permalink raw reply
* [PATCH 2/2] diff: add 'diff.wordDiff' config option
From: Karthik Nayak @ 2024-03-02 9:57 UTC (permalink / raw)
To: oliver; +Cc: git, Karthik Nayak
In-Reply-To: <20240302095751.123138-1-karthik.188@gmail.com>
The git-diff(1) command supports the `--word-diff` which allows the
users to specify how to delimit word diffs. Provide this option also as
a config param 'diff.wordDiff'.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
Documentation/config/diff.txt | 4 +++
diff.c | 50 +++++++++++++++++++++++++++--------
t/t4034-diff-words.sh | 11 ++++++--
3 files changed, 52 insertions(+), 13 deletions(-)
diff --git a/Documentation/config/diff.txt b/Documentation/config/diff.txt
index bd5ae0c337..00459beee2 100644
--- a/Documentation/config/diff.txt
+++ b/Documentation/config/diff.txt
@@ -225,3 +225,7 @@ diff.colorMovedWS::
When moved lines are colored using e.g. the `diff.colorMoved` setting,
this option controls the `<mode>` how spaces are treated
for details of valid modes see '--color-moved-ws' in linkgit:git-diff[1].
+
+diff.wordDiff::
+ Show a word diff, using the `<mode>` to delimit changed words.
+ For details of valid modes see '--word-diff' in linkgit:git-diff[1].
diff --git a/diff.c b/diff.c
index e50def4538..050d83ef85 100644
--- a/diff.c
+++ b/diff.c
@@ -69,6 +69,7 @@ static int diff_dirstat_permille_default = 30;
static struct diff_options default_diff_options;
static long diff_algorithm;
static unsigned ws_error_highlight_default = WSEH_NEW;
+static int diff_word_diff_default;
static char diff_colors[][COLOR_MAXLEN] = {
GIT_COLOR_RESET,
@@ -209,6 +210,23 @@ int git_config_rename(const char *var, const char *value)
return git_config_bool(var,value) ? DIFF_DETECT_RENAME : 0;
}
+static int parse_word_diff_value(const char *value)
+{
+ if (!value)
+ return -1;
+ else if (!strcmp(value, "plain"))
+ return DIFF_WORDS_PLAIN;
+ else if (!strcmp(value, "color")) {
+ return DIFF_WORDS_COLOR;
+ }
+ else if (!strcmp(value, "porcelain"))
+ return DIFF_WORDS_PORCELAIN;
+ else if (!strcmp(value, "none"))
+ return DIFF_WORDS_NONE;
+
+ return -1;
+}
+
long parse_algorithm_value(const char *value)
{
if (!value)
@@ -452,6 +470,18 @@ int git_diff_ui_config(const char *var, const char *value,
return 0;
}
+ if (!strcmp(var, "diff.worddiff")) {
+ if (!value)
+ return config_error_nonbool(var);
+ diff_word_diff_default = parse_word_diff_value(value);
+ if (diff_word_diff_default < 0)
+ return error(_("unknown value for config '%s': %s"),
+ var, value);
+ if (diff_word_diff_default == DIFF_WORDS_COLOR)
+ diff_use_color_default = 1;
+ return 0;
+ }
+
if (git_color_config(var, value, cb) < 0)
return -1;
@@ -4724,6 +4754,7 @@ void repo_diff_setup(struct repository *r, struct diff_options *options)
options->use_color = diff_use_color_default;
options->detect_rename = diff_detect_rename_default;
options->xdl_opts |= diff_algorithm;
+ options->word_diff |= diff_word_diff_default;
if (diff_indent_heuristic)
DIFF_XDL_SET(options, INDENT_HEURISTIC);
@@ -5504,21 +5535,18 @@ static int diff_opt_word_diff(const struct option *opt,
const char *arg, int unset)
{
struct diff_options *options = opt->value;
+ int value;
BUG_ON_OPT_NEG(unset);
+
if (arg) {
- if (!strcmp(arg, "plain"))
- options->word_diff = DIFF_WORDS_PLAIN;
- else if (!strcmp(arg, "color")) {
- options->use_color = 1;
- options->word_diff = DIFF_WORDS_COLOR;
- }
- else if (!strcmp(arg, "porcelain"))
- options->word_diff = DIFF_WORDS_PORCELAIN;
- else if (!strcmp(arg, "none"))
- options->word_diff = DIFF_WORDS_NONE;
- else
+ value = parse_word_diff_value(arg);
+ if (value < 0)
return error(_("bad --word-diff argument: %s"), arg);
+ if (value == DIFF_WORDS_COLOR)
+ options->use_color = 1;
+
+ options->word_diff = value;
} else {
if (options->word_diff == DIFF_WORDS_NONE)
options->word_diff = DIFF_WORDS_PLAIN;
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 4f70aa6e9f..2cc391c339 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -56,12 +56,19 @@ diff_with_opts () {
post=$(git rev-parse --short $(git hash-object post)) &&
test_must_fail git diff --no-index "$@" pre post >output &&
test_decode_color <output >output.decrypted &&
- sed -e "2s/index [^ ]*/index $pre..$post/" expect >expected
+ sed -e "2s/index [^ ]*/index $pre..$post/" expect >expected &&
test_cmp expected output.decrypted
}
word_diff () {
- diff_with_opts "--word-diff=$1" $(echo "$@" | cut -d' ' -s -f 2-)
+ # Capture the rest of the arguments to passthrough.
+ rest=$(echo "$@" | cut -d' ' -s -f 2-) &&
+ # Test via the config route.
+ git config diff.wordDiff $1 &&
+ diff_with_opts $rest &&
+ git config --unset diff.wordDiff &&
+ # Test via the command option route.
+ diff_with_opts "--word-diff=$1" $rest
}
test_language_driver () {
--
2.43.GIT
^ permalink raw reply related
* [PATCH 1/2] t4034: extract out `diff_with_opts`
From: Karthik Nayak @ 2024-03-02 9:57 UTC (permalink / raw)
To: oliver; +Cc: git, Karthik Nayak
In-Reply-To: <20240302095751.123138-1-karthik.188@gmail.com>
The current `word_diff` function in t4034 uses the arguments provided to
obtain the diff and compare them.
Let's rename this to `diff_with_opts` and add a new function `word_diff`
which is specific to the `--word-diff=` option. This function will act
as a wrapper around `diff_with_opts`.
In the following commit, when we introduce a config option for
`--word-diff`, we'd want to test the existing tests also with the config
option and at this point we can expand `word_diff` to test both config
and command line options.
Signed-off-by: Karthik Nayak <karthik.188@gmail.com>
---
t/t4034-diff-words.sh | 54 +++++++++++++++++++++++--------------------
1 file changed, 29 insertions(+), 25 deletions(-)
diff --git a/t/t4034-diff-words.sh b/t/t4034-diff-words.sh
index 74586f3813..4f70aa6e9f 100755
--- a/t/t4034-diff-words.sh
+++ b/t/t4034-diff-words.sh
@@ -51,7 +51,7 @@ cat >expect.non-whitespace-is-word <<-EOF
<GREEN>aeff = aeff * ( aaa )<RESET>
EOF
-word_diff () {
+diff_with_opts () {
pre=$(git rev-parse --short $(git hash-object pre)) &&
post=$(git rev-parse --short $(git hash-object post)) &&
test_must_fail git diff --no-index "$@" pre post >output &&
@@ -60,6 +60,10 @@ word_diff () {
test_cmp expected output.decrypted
}
+word_diff () {
+ diff_with_opts "--word-diff=$1" $(echo "$@" | cut -d' ' -s -f 2-)
+}
+
test_language_driver () {
lang=$1
test_expect_success "diff driver '$lang'" '
@@ -67,11 +71,11 @@ test_language_driver () {
"$TEST_DIRECTORY/t4034/'"$lang"'/post" \
"$TEST_DIRECTORY/t4034/'"$lang"'/expect" . &&
echo "* diff='"$lang"'" >.gitattributes &&
- word_diff --color-words
+ diff_with_opts --color-words
'
test_expect_success "diff driver '$lang' in Islandic" '
LANG=is_IS.UTF-8 LANGUAGE=is LC_ALL="$is_IS_locale" \
- word_diff --color-words
+ diff_with_opts --color-words
'
}
@@ -81,7 +85,7 @@ test_expect_success setup '
git config diff.color.func magenta
'
-test_expect_success 'set up pre and post with runs of whitespace' '
+test_expect_success 'setup pre and post with runs of whitespace' '
cp pre.simple pre &&
cp post.simple post
'
@@ -101,9 +105,9 @@ test_expect_success 'word diff with runs of whitespace' '
<GREEN>aeff = aeff * ( aaa )<RESET>
EOF
- word_diff --color-words &&
- word_diff --word-diff=color &&
- word_diff --color --word-diff=color
+ diff_with_opts --color-words &&
+ word_diff color &&
+ diff_with_opts --color --word-diff=color
'
test_expect_success '--word-diff=porcelain' '
@@ -127,7 +131,7 @@ test_expect_success '--word-diff=porcelain' '
+aeff = aeff * ( aaa )
~
EOF
- word_diff --word-diff=porcelain
+ word_diff porcelain
'
test_expect_success '--word-diff=plain' '
@@ -145,8 +149,8 @@ test_expect_success '--word-diff=plain' '
{+aeff = aeff * ( aaa )+}
EOF
- word_diff --word-diff=plain &&
- word_diff --word-diff=plain --no-color
+ diff_with_opts --word-diff=plain &&
+ word_diff plain --no-color
'
test_expect_success '--word-diff=plain --color' '
@@ -164,7 +168,7 @@ test_expect_success '--word-diff=plain --color' '
<GREEN>{+aeff = aeff * ( aaa )+}<RESET>
EOF
- word_diff --word-diff=plain --color
+ word_diff plain --color
'
test_expect_success 'word diff without context' '
@@ -181,17 +185,17 @@ test_expect_success 'word diff without context' '
<GREEN>aeff = aeff * ( aaa )<RESET>
EOF
- word_diff --color-words --unified=0
+ diff_with_opts --color-words --unified=0
'
test_expect_success 'word diff with a regular expression' '
cp expect.letter-runs-are-words expect &&
- word_diff --color-words="[a-z]+"
+ diff_with_opts --color-words="[a-z]+"
'
test_expect_success 'word diff with zero length matches' '
cp expect.letter-runs-are-words expect &&
- word_diff --color-words="[a-z${LF}]*"
+ diff_with_opts --color-words="[a-z${LF}]*"
'
test_expect_success 'set up a diff driver' '
@@ -204,12 +208,12 @@ test_expect_success 'set up a diff driver' '
test_expect_success 'option overrides .gitattributes' '
cp expect.letter-runs-are-words expect &&
- word_diff --color-words="[a-z]+"
+ diff_with_opts --color-words="[a-z]+"
'
test_expect_success 'use regex supplied by driver' '
cp expect.non-whitespace-is-word expect &&
- word_diff --color-words
+ diff_with_opts --color-words
'
test_expect_success 'set up diff.wordRegex option' '
@@ -218,7 +222,7 @@ test_expect_success 'set up diff.wordRegex option' '
test_expect_success 'command-line overrides config' '
cp expect.letter-runs-are-words expect &&
- word_diff --color-words="[a-z]+"
+ diff_with_opts --color-words="[a-z]+"
'
test_expect_success 'command-line overrides config: --word-diff-regex' '
@@ -236,12 +240,12 @@ test_expect_success 'command-line overrides config: --word-diff-regex' '
<GREEN>{+aeff = aeff * ( aaa+}<RESET> )
EOF
- word_diff --color --word-diff-regex="[a-z]+"
+ diff_with_opts --color --word-diff-regex="[a-z]+"
'
test_expect_success '.gitattributes override config' '
cp expect.non-whitespace-is-word expect &&
- word_diff --color-words
+ diff_with_opts --color-words
'
test_expect_success 'setup: remove diff driver regex' '
@@ -263,7 +267,7 @@ test_expect_success 'use configured regex' '
<GREEN>aeff = aeff * ( aaa<RESET> )
EOF
- word_diff --color-words
+ diff_with_opts --color-words
'
test_expect_success 'test parsing words for newline' '
@@ -279,7 +283,7 @@ test_expect_success 'test parsing words for newline' '
<CYAN>@@ -1 +1 @@<RESET>
aaa (aaa) <GREEN>aaa<RESET>
EOF
- word_diff --color-words="a+"
+ diff_with_opts --color-words="a+"
'
test_expect_success 'test when words are only removed at the end' '
@@ -295,7 +299,7 @@ test_expect_success 'test when words are only removed at the end' '
<CYAN>@@ -1 +1 @@<RESET>
(<RED>:<RESET>
EOF
- word_diff --color-words=.
+ diff_with_opts --color-words=.
'
test_expect_success '--word-diff=none' '
@@ -312,7 +316,7 @@ test_expect_success '--word-diff=none' '
-(:
+(
EOF
- word_diff --word-diff=plain --word-diff=none
+ word_diff plain --word-diff=none
'
test_expect_success 'unset default driver' '
@@ -363,7 +367,7 @@ test_expect_success 'word-diff with diff.sbe' '
[-b-]{+c+}
EOF
test_config diff.suppress-blank-empty true &&
- word_diff --word-diff=plain
+ word_diff plain
'
test_expect_success 'word-diff with no newline at EOF' '
@@ -379,7 +383,7 @@ test_expect_success 'word-diff with no newline at EOF' '
@@ -1 +1 @@
a a [-a-]{+ab+} a a
EOF
- word_diff --word-diff=plain
+ word_diff plain
'
test_expect_success 'setup history with two files' '
--
2.43.GIT
^ permalink raw reply related
* [PATCH 0/2] Support diff.wordDiff config
From: Karthik Nayak @ 2024-03-02 9:57 UTC (permalink / raw)
To: oliver; +Cc: git, Karthik Nayak
In-Reply-To: <a7be415d-5005-4fa7-9b2e-1974b7439a81@schinagl.nl>
This patch series adds the diff.wordDiff config option. This mimics the
'--word-diff' option of `git-diff(1)`.
The first patch is more of a preparatory patch, which makes it easier to
add tests when the actual config is added in patch 2.
Karthik Nayak (2):
t4034: extract out `diff_with_opts`
diff: add 'diff.wordDiff' config option
Documentation/config/diff.txt | 4 +++
diff.c | 50 +++++++++++++++++++++------
t/t4034-diff-words.sh | 63 ++++++++++++++++++++---------------
3 files changed, 80 insertions(+), 37 deletions(-)
--
2.43.GIT
^ permalink raw reply
* Re: [PATCH] test-lib-functions: simplify `test_file_not_empty` failure message
From: Dirk Gouders @ 2024-03-02 7:07 UTC (permalink / raw)
To: Eric Sunshine; +Cc: Junio C Hamano, Eric Sunshine, git, Aryan Gupta
In-Reply-To: <CAPig+cSt4Q0e+DioFiy7yjBgn5+gWNqQOYc0eCAgdYrZUp8VBQ@mail.gmail.com>
Eric Sunshine <sunshine@sunshineco.com> writes:
> On Fri, Mar 1, 2024 at 5:11 PM Junio C Hamano <gitster@pobox.com> wrote:
>> Eric Sunshine <ericsunshine@charter.net> writes:
>> > A more accurate message might be "'foo' is empty but
>> > should not be (or doesn't exist)", but that's unnecessarily long-winded
>> > and adds little information that the test author couldn't discover by
>> > noticing the file's absence.
>>
>> The "adds little information" version may be
>>
>> echo "'$1' is either missing or empty, but should not be"
>>
>> And avoiding "X is Y, but should be ~Y" construct, perhaps
>>
>> echo "'$1' should be a file with non-empty contents"
>>
>> would work better? I dunno.
>
> I find "'$1' is either missing or empty, but should not be" suggestion
> clear and easily understood. I'll reroll with that.
This is a view from a position with more distance:
I find that not so easily understood -- the "but should not
be" part is rather unexpected and I feel, it doesn't provide necessary
information, e.g.:
test_path_is_executable () {
...
echo "$1 is not executable"
...
also doesn't state what is wanted and I doubt that message doesn't
clearly describe the problem.
While I looked at it: there is another `test -s` in test_grep () that
perhaps could be fixed the same way:
if test -s "$last_arg"
then
cat >&4 "$last_arg"
else
echo >&4 "<File '$last_arg' is empty>"
fi
Dirk
^ permalink raw reply
* Re: [PATCH] test-lib-functions: simplify `test_file_not_empty` failure message
From: Eric Sunshine @ 2024-03-01 22:59 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Eric Sunshine, git, Aryan Gupta
In-Reply-To: <xmqqsf194n7a.fsf@gitster.g>
On Fri, Mar 1, 2024 at 5:11 PM Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <ericsunshine@charter.net> writes:
> > A more accurate message might be "'foo' is empty but
> > should not be (or doesn't exist)", but that's unnecessarily long-winded
> > and adds little information that the test author couldn't discover by
> > noticing the file's absence.
>
> The "adds little information" version may be
>
> echo "'$1' is either missing or empty, but should not be"
>
> And avoiding "X is Y, but should be ~Y" construct, perhaps
>
> echo "'$1' should be a file with non-empty contents"
>
> would work better? I dunno.
I find "'$1' is either missing or empty, but should not be" suggestion
clear and easily understood. I'll reroll with that.
^ permalink raw reply
* Re: [PATCH] test-lib-functions: simplify `test_file_not_empty` failure message
From: Junio C Hamano @ 2024-03-01 22:11 UTC (permalink / raw)
To: Eric Sunshine; +Cc: git, Aryan Gupta, Eric Sunshine
In-Reply-To: <20240301204922.40304-1-ericsunshine@charter.net>
Eric Sunshine <ericsunshine@charter.net> writes:
> Note: Technically, the revised message is slightly less accurate since
> the function asserts both that the file exists and that it is non-empty,
> but the new message talks only about the emptiness of the file, not
> whether it exists.
>
> A more accurate message might be "'foo' is empty but
> should not be (or doesn't exist)", but that's unnecessarily long-winded
> and adds little information that the test author couldn't discover by
> noticing the file's absence.
Besides, that is way too confusing. "<foo> is empty or it does not
exist" I may understand, but with your construct, I wouldn't be able
to tell how I am supposed to interpret the "(or doesn't exist)"
part.
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index b5eaf7fdc1..9e97b324c5 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -991,7 +991,7 @@ test_file_not_empty () {
> test "$#" = 2 && BUG "2 param"
> if ! test -s "$1"
> then
> - echo "'$1' is not a non-empty file."
> + echo "'$1' is empty but should not be"
The "adds little information" version may be
echo "'$1' is either missing or empty, but should not be"
And avoiding "X is Y, but should be ~Y" construct, perhaps
echo "'$1' should be a file with non-empty contents"
would work better? I dunno.
^ permalink raw reply
* [PATCH] test-lib-functions: simplify `test_file_not_empty` failure message
From: Eric Sunshine @ 2024-03-01 20:49 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Aryan Gupta, Eric Sunshine
From: Eric Sunshine <sunshine@sunshineco.com>
The function `test_file_not_empty` asserts that a file exists and is not
empty. When the assertion fails, it complains:
'foo' is not a non-empty file.
which is difficult to interpret due to the double-negative. To make it
easier to understand the problem, simplify the message by dropping the
double-negative and stating the problem more directly:
'foo' is empty but should not be
(The full-stop is also dropped from the message to reflect the style of
messages issued by other `test_path_*` functions.)
Note: Technically, the revised message is slightly less accurate since
the function asserts both that the file exists and that it is non-empty,
but the new message talks only about the emptiness of the file, not
whether it exists. A more accurate message might be "'foo' is empty but
should not be (or doesn't exist)", but that's unnecessarily long-winded
and adds little information that the test author couldn't discover by
noticing the file's absence.
Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
This is a tangential follow-up to the discussion at [1].
[1]: https://lore.kernel.org/git/CAPig+cQ+JNBwydUq0CsTZGs8mHs3L3fJDuSosd+-WdKwWWw=gg@mail.gmail.com/
t/test-lib-functions.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index b5eaf7fdc1..9e97b324c5 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -991,7 +991,7 @@ test_file_not_empty () {
test "$#" = 2 && BUG "2 param"
if ! test -s "$1"
then
- echo "'$1' is not a non-empty file."
+ echo "'$1' is empty but should not be"
false
fi
}
--
2.44.0
^ permalink raw reply related
* Re: [PATCH] clean: improve -n and -f implementation and documentation
From: Sergey Organov @ 2024-03-01 19:31 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jean-Noël Avila, git
In-Reply-To: <xmqqmsrh6d2f.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Jean-Noël Avila <avila.jn@gmail.com> writes:
>
>>> + /* Dry run won't remove anything, so requiring force makes no sense */
>>> + if(dry_run)
>>> + require_force = 0;
>
> Style. "if (dry_run)".
Ooops!
>
> Getting rid of "config_set", which was an extra variable that kept
> track of where "force" came from, does make the logic cleaner, I
> guess. What we want to happen is that one of -i/-n/-f is required
> when clean.requireForce is *not* unset (i.e. 0 <= require_force).
>
>>> + if (!force && !interactive) {
>
> The require-force takes effect only when neither force or
> interactive is given, so the new code structure puts the above
> obvious conditional around "do we complain due to requireForce?"
> logic. Sensible.
>
>>> + if (require_force > 0)
>>> + die(_("clean.requireForce set to true and neither -f, nor -i given; "
>>> + "refusing to clean"));
>
> If it is explicitly set, we get this message. And ...
>
>>> + else if (require_force < 0)
>>> + die(_("clean.requireForce defaults to true and neither -f, nor -i given; "
>>> "refusing to clean"));
>
> ... if it is set due to default (in other words, if it is not unset), we
> get this message.
>
> As you said, I do not think it matters too much either way to the
> end-users where the truth setting of clean.requireForce came from,
> either due to the default or the user explicitly configuring. So
> unifying to a single message may be helpful to both readers and
> translators.
>
> clean.requireForce is true; unless interactive, -f is required
>
> might be a bit shorter and more to the point.
Dunno, I tried to keep changes to the bare sensible minimum, especially
to avoid possible controversy. I'd leave this for somebody else to
decide upon and patch, if they feel like it.
>> The last two cases can be coalesced into a single case (the last one),
>> because the difference in the messages does not bring more information
>> to the user.
>
> Yeah.
"The last two cases" sounds like there are more of them, and there is
none, so to me it sounded like patch misread. Maybe I'm wrong.
Thanks for the review!
-- Sergey Organov
^ permalink raw reply
* [ANNOUNCE] Git Rev News edition 108
From: Christian Couder @ 2024-03-01 19:16 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Jakub Narebski, Markus Jansen,
Štěpán Němec, Kaartic Sivaraam, Taylor Blau,
Johannes Schindelin, Ævar Arnfjörð Bjarmason, lwn,
Bruno Brito, René Scharfe, Jeff King, Dragan Simic,
Brooke Kuhlmann, Jack Lot
Hi everyone,
The 108th edition of Git Rev News is now published:
https://git.github.io/rev_news/2024/02/29/edition-108/
Thanks a lot to Brooke Kuhlmann, Jack Lot, Štěpán Němec and Bruno
Brito who helped this month!
Enjoy,
Christian, Jakub, Markus and Kaartic.
PS: An issue for the next edition is already opened and contributions
are welcome:
https://github.com/git/git.github.io/issues/697
^ permalink raw reply
* Support with finding git objects
From: Ayoub, Toufic @ 2024-03-01 19:06 UTC (permalink / raw)
To: git@vger.kernel.org
Hello everyone,
Hope this message finds you. I need your support regarding an issue my team is facing.
We have a code in our SW which shall display the date of the git commit. There is a path in the code that if it is executed on a platform where git is not installed, that this commit date shall be found by looking at:
1. Reading from HEAD file the commit hash
2. Searching for this commit hash under .git/objects
The problem we are facing is that sometimes, when the default master branch is updated with the newest commit, this commit will not be found locally, even after doing git pull and so on...
The commit can be found with:
1. git log
2. git cat-file -t <commit hash> -> it returns "commit"
3. git reflog
but the strange thing is that we can neither find it under .git/objects/ nor under .git/objects/pack (although that is the newest commit and shall not be packed).
Can you please provide support by:
1. telling me it that is an issue or is it expected?
2. If that is expected, where shall we find this commit object to retrieve the commit message and date from it (without running git commands)?
3. Or recommending me some other solution
I would appreciate that.
Best regards,
Toufic Ayoub
Software Developer
Hemodynamic Algorithms
Hospital Patient Monitoring
Philips
Philips Medizin Systeme Böblingen GmbH, Hewlett-Packard-Strasse 2, 71034 Boeblingen, Germany
Email toufic.ayoub@philips.com |www.philips.com/healthcare
Geschäftsführer: Dr. Dieter Haase (Sprecher), Günter Gegner, Sang Do Kim, Florian Kuhn
Registergericht Stuttgart Reg.-Nr. HRB 245187, Sitz der Gesellschaft: Böblingen
Simply switch to printing double-sided and printing less.
________________________________
The information contained in this message may be confidential and legally protected under applicable law. The message is intended solely for the addressee(s). If you are not the intended recipient, you are hereby notified that any use, forwarding, dissemination, or reproduction of this message is strictly prohibited and may be unlawful. If you are not the intended recipient, please contact the sender by return e-mail and destroy all copies of the original message.
^ permalink raw reply
* Re: [PATCH] gitcli: drop mention of “non-dashed form”
From: Junio C Hamano @ 2024-03-01 18:35 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git, Johannes Schindelin
In-Reply-To: <5b34bc4e22816f7f19bd26c15a08fe4c749b72f8.1709316230.git.code@khaugsbakk.name>
Kristoffer Haugsbakk <code@khaugsbakk.name> writes:
> ... These days though it seems like an irrelevant point to make to
> budding CLI scripters—you don’t have to warn against a style that
> probably doesn’t even work on their git(1) installation.
It is very true. It is still technically correct that you should
prefer spelling "git cat-file" instead of "git-cat-file", as the
latter may not work for you at all ;-).
^ permalink raw reply
* Re: [PATCH] branch: advise about ref syntax rules
From: Junio C Hamano @ 2024-03-01 18:32 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: git
In-Reply-To: <1ba698b2-a0da-4d62-8174-0ee6d6cd9bbc@app.fastmail.com>
"Kristoffer Haugsbakk" <code@khaugsbakk.name> writes:
> I thought of doing that, but I reckoned that people who have a good
> intuition for the ref syntax would not get this error enough to want to
> turn if off.
If that is your choice, that is perfectly OK, as long as the
proposed log message clearly records why we did not bother using
advice_if_enabled().
If that is the case, then a rewrite for existing die() would become:
int code = die_message(_("'%s' is not a valid branch name"), name);
advise(_("See `man git check-ref-format`"));
exit(code);
Thanks.
^ permalink raw reply
* Re: [PATCH] clean: improve -n and -f implementation and documentation
From: Junio C Hamano @ 2024-03-01 18:30 UTC (permalink / raw)
To: Jean-Noël Avila; +Cc: Sergey Organov, git
In-Reply-To: <xmqqmsrh6d2f.fsf@gitster.g>
Junio C Hamano <gitster@pobox.com> writes:
> Getting rid of "config_set", which was an extra variable that kept
> track of where "force" came from, does make the logic cleaner, I
> guess. What we want to happen is that one of -i/-n/-f is required
> when clean.requireForce is *not* unset (i.e. 0 <= require_force).
Oh, noes. require_force is unset explicitly it would be 0, so this
should have read (i.e. require_force != 0). Sorry for a thinko.
^ permalink raw reply
* RE: [PATCH v3] build: support z/OS (OS/390).
From: rsbecker @ 2024-03-01 18:25 UTC (permalink / raw)
To: 'Junio C Hamano', 'Haritha D'
Cc: git, 'Kristoffer Haugsbakk'
In-Reply-To: <xmqqbk7x6cox.fsf@gitster.g>
On Friday, March 1, 2024 1:15 PM, Junio C Hamano wrote:
>Haritha D <Harithamma.D@ibm.com> writes:
>
>> The win test(7) test case failed stating the reason as "The Operation
>> cancelled". I saw that it failed after 5 hours 59 minutes of running
>> the test cases(build). How do I handle this?
>
>Those "win test (n)", at least some of them, seem to have been somewhat
flaky [*1*]. If you are certain you did not break them with
>your change, you do not have to fix them yourself.
>
>I am a wrong person to ask how the test failure that may [*2*] block GGG
submission can be circumvented, as I am not involved in that
>machinery at all.
>
>Thanks.
>
>
>[Footnotes]
>
>*1* Also I've seen osx-clang job time-out from time to time, without
> failing any specific test. Re-running failed jobs from the menu
> often make them pass, which is why I said "somewhat flaky".
>
>*2* I do not even know if GGG refuses to submit a series with a test
> failure, let alone if it allows to override such a safety if
> exists.
Which tests have been hanging on S390? We have occasional hangs on NonStop
that end up being attributed to our CI build system not supplying pipes
properly to git. It would be interesting if the same tests are having issues
on different platforms.
--Randall
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox