Volume XXII, number 279Tuesday, October 6, 2026Latest message 13 minutes ago

The Git List

News and archive of git@vger.kernel.org, since April 2005

patchsequencer: remove unnecessary variable setting

3 messages between Aug 12, 2026 and Aug 13, 2026, from Elijah Newren via GitGitGadget, Junio C Hamano, Patrick Steinhardt.

Plain Markdown or JSON for tools and agents. Diffs are folded; open one to read it.

Elijah Newren via GitGitGadgetAug 12, 2026, 06:42 UTC on lore
From: Elijah Newren <newren@gmail.com>

revs.pretty_given is only ever read in builtin/log.c, and nothing from builtin/log.c is ever called from sequencer.c. So setting this variable cannot do anything.

This was introduced in commit 62db524779 ("rebase -i: generate the script via rebase--helper", 2017-07-14), which used `git rev-list` even though its commit message describes the logic as having been based on `git log`. Because of this, I am guessing this line was copied or ported from part of builtin/log.c without recognizing that this line was not doing anything and could be removed.

It's certainly not doing anything now, though, so remove it.
Signed-off-by: Elijah Newren <newren@gmail.com>
---
    sequencer: remove unnecessary variable setting
    
    Random thing I noticed a few years ago, I believe while investigating
    our tangled web of revision fields and parsing. Either way, it's still
    valid and I'm finally sending it upstream.
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1922%2Fnewren%2Fsequencer-remove-unnecessary-setting-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1922/newren/sequencer-remove-unnecessary-setting-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/1922
 sequencer.c | 1 -
 1 file changed, 1 deletion(-)
Show changes to sequencer.c +0 −1
diff --git a/sequencer.c b/sequencer.c
index 83c3849205..a0abcc69ce 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6277,7 +6277,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out,
 	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
 	revs.topo_order = 1;
 
-	revs.pretty_given = 1;
 	repo_config_get_string(the_repository, "rebase.instructionFormat", &format);
 	if (!format || !*format) {
 		free(format);

base-commit: 2c78326f810173a4f3aefd8021f1e07575412481
-- 
gitgitgadget
Junio C HamanoAug 12, 2026, 17:24 UTC in reply to Elijah Newren via GitGitGadget on lore

Re: [PATCH] sequencer: remove unnecessary variable setting

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 5 quoted lines
> From: Elijah Newren <newren@gmail.com>
>
> revs.pretty_given is only ever read in builtin/log.c, and nothing from
> builtin/log.c is ever called from sequencer.c.  So setting this variable
> cannot do anything.
Thanks.  I'll mark the topic for 'next'.
Show 38 quoted lines
> This was introduced in commit 62db524779 ("rebase -i: generate the
> script via rebase--helper", 2017-07-14), which used `git rev-list` even
> though its commit message describes the logic as having been based on
> `git log`.  Because of this, I am guessing this line was copied or
> ported from part of builtin/log.c without recognizing that this line was
> not doing anything and could be removed.
>
> It's certainly not doing anything now, though, so remove it.
>
> Signed-off-by: Elijah Newren <newren@gmail.com>
> ---
>     sequencer: remove unnecessary variable setting
>     
>     Random thing I noticed a few years ago, I believe while investigating
>     our tangled web of revision fields and parsing. Either way, it's still
>     valid and I'm finally sending it upstream.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1922%2Fnewren%2Fsequencer-remove-unnecessary-setting-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1922/newren/sequencer-remove-unnecessary-setting-v1
> Pull-Request: https://github.com/gitgitgadget/git/pull/1922
>
>  sequencer.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 83c3849205..a0abcc69ce 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6277,7 +6277,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out,
>  	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
>  	revs.topo_order = 1;
>  
> -	revs.pretty_given = 1;
>  	repo_config_get_string(the_repository, "rebase.instructionFormat", &format);
>  	if (!format || !*format) {
>  		free(format);
>
> base-commit: 2c78326f810173a4f3aefd8021f1e07575412481
Patrick SteinhardtAug 13, 2026, 07:44 UTC in reply to Elijah Newren via GitGitGadget on lore

Re: [PATCH] sequencer: remove unnecessary variable setting

On Wed, Aug 12, 2026 at 06:42:38AM +0000, Elijah Newren via GitGitGadget wrote:
Show 12 quoted lines
> diff --git a/sequencer.c b/sequencer.c
> index 83c3849205..a0abcc69ce 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6277,7 +6277,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out,
>  	revs.sort_order = REV_SORT_IN_GRAPH_ORDER;
>  	revs.topo_order = 1;
>  
> -	revs.pretty_given = 1;
>  	repo_config_get_string(the_repository, "rebase.instructionFormat", &format);
>  	if (!format || !*format) {
>  		free(format);

Makes sense. The only reference to this field is indeed in "builtin/log.c", and as we don't use the sequencer there shouldn't be any kind of interaction between those two subsystems here.

Thanks!
Patrick

Back to recent threads