{"thread":{"id":"66162","subject":"[PATCH] sequencer: remove unnecessary variable setting","startedAt":"2026-08-12T06:42:41Z","lastAt":"2026-08-13T07:44:21Z","messageCount":3,"participants":["Elijah Newren via GitGitGadget","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"550349","messageId":"pull.1922.git.1786516959130.gitgitgadget@gmail.com","threadId":"66162","inReplyTo":null,"subject":"[PATCH] sequencer: remove unnecessary variable setting","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-12T06:42:38Z","receivedAt":"2026-08-12T06:42:41Z","isPatch":true,"body":"From: Elijah Newren <newren@gmail.com>\n\nrevs.pretty_given is only ever read in builtin/log.c, and nothing from\nbuiltin/log.c is ever called from sequencer.c.  So setting this variable\ncannot do anything.\n\nThis was introduced in commit 62db524779 (\"rebase -i: generate the\nscript via rebase--helper\", 2017-07-14), which used `git rev-list` even\nthough its commit message describes the logic as having been based on\n`git log`.  Because of this, I am guessing this line was copied or\nported from part of builtin/log.c without recognizing that this line was\nnot doing anything and could be removed.\n\nIt's certainly not doing anything now, though, so remove it.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n    sequencer: remove unnecessary variable setting\n    \n    Random thing I noticed a few years ago, I believe while investigating\n    our tangled web of revision fields and parsing. Either way, it's still\n    valid and I'm finally sending it upstream.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-1922%2Fnewren%2Fsequencer-remove-unnecessary-setting-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1922/newren/sequencer-remove-unnecessary-setting-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/1922\n\n sequencer.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/sequencer.c b/sequencer.c\nindex 83c3849205..a0abcc69ce 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -6277,7 +6277,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out,\n \trevs.sort_order = REV_SORT_IN_GRAPH_ORDER;\n \trevs.topo_order = 1;\n \n-\trevs.pretty_given = 1;\n \trepo_config_get_string(the_repository, \"rebase.instructionFormat\", &format);\n \tif (!format || !*format) {\n \t\tfree(format);\n\nbase-commit: 2c78326f810173a4f3aefd8021f1e07575412481\n-- \ngitgitgadget\n"},{"id":"550432","messageId":"xmqqa4qrxneq.fsf@gitster.g","threadId":"66162","inReplyTo":"pull.1922.git.1786516959130.gitgitgadget@gmail.com","subject":"Re: [PATCH] sequencer: remove unnecessary variable setting","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-12T17:24:13Z","receivedAt":"2026-08-12T17:24:15Z","isPatch":true,"body":"\"Elijah Newren via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: Elijah Newren <newren@gmail.com>\n>\n> revs.pretty_given is only ever read in builtin/log.c, and nothing from\n> builtin/log.c is ever called from sequencer.c.  So setting this variable\n> cannot do anything.\n\nThanks.  I'll mark the topic for 'next'.\n\n> This was introduced in commit 62db524779 (\"rebase -i: generate the\n> script via rebase--helper\", 2017-07-14), which used `git rev-list` even\n> though its commit message describes the logic as having been based on\n> `git log`.  Because of this, I am guessing this line was copied or\n> ported from part of builtin/log.c without recognizing that this line was\n> not doing anything and could be removed.\n>\n> It's certainly not doing anything now, though, so remove it.\n>\n> Signed-off-by: Elijah Newren <newren@gmail.com>\n> ---\n>     sequencer: remove unnecessary variable setting\n>     \n>     Random thing I noticed a few years ago, I believe while investigating\n>     our tangled web of revision fields and parsing. Either way, it's still\n>     valid and I'm finally sending it upstream.\n>\n> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1922%2Fnewren%2Fsequencer-remove-unnecessary-setting-v1\n> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1922/newren/sequencer-remove-unnecessary-setting-v1\n> Pull-Request: https://github.com/gitgitgadget/git/pull/1922\n>\n>  sequencer.c | 1 -\n>  1 file changed, 1 deletion(-)\n>\n> diff --git a/sequencer.c b/sequencer.c\n> index 83c3849205..a0abcc69ce 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -6277,7 +6277,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out,\n>  \trevs.sort_order = REV_SORT_IN_GRAPH_ORDER;\n>  \trevs.topo_order = 1;\n>  \n> -\trevs.pretty_given = 1;\n>  \trepo_config_get_string(the_repository, \"rebase.instructionFormat\", &format);\n>  \tif (!format || !*format) {\n>  \t\tfree(format);\n>\n> base-commit: 2c78326f810173a4f3aefd8021f1e07575412481\n"},{"id":"550477","messageId":"an11zsTm-fanH8yt@pks.im","threadId":"66162","inReplyTo":"pull.1922.git.1786516959130.gitgitgadget@gmail.com","subject":"Re: [PATCH] sequencer: remove unnecessary variable setting","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-13T07:44:14Z","receivedAt":"2026-08-13T07:44:21Z","isPatch":true,"body":"On Wed, Aug 12, 2026 at 06:42:38AM +0000, Elijah Newren via GitGitGadget wrote:\n> diff --git a/sequencer.c b/sequencer.c\n> index 83c3849205..a0abcc69ce 100644\n> --- a/sequencer.c\n> +++ b/sequencer.c\n> @@ -6277,7 +6277,6 @@ int sequencer_make_script(struct repository *r, struct strbuf *out,\n>  \trevs.sort_order = REV_SORT_IN_GRAPH_ORDER;\n>  \trevs.topo_order = 1;\n>  \n> -\trevs.pretty_given = 1;\n>  \trepo_config_get_string(the_repository, \"rebase.instructionFormat\", &format);\n>  \tif (!format || !*format) {\n>  \t\tfree(format);\n\nMakes sense. The only reference to this field is indeed in\n\"builtin/log.c\", and as we don't use the sequencer there shouldn't be\nany kind of interaction between those two subsystems here.\n\nThanks!\n\nPatrick\n"}]}