From: Junio C Hamano <gitster@pobox.com>
To: "Sean Allred via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Sean Allred <code@seanallred.com>,
Sean Allred <allred.sean@gmail.com>
Subject: Re: [PATCH] var: add GIT_SEQUENCE_EDITOR variable
Date: Mon, 21 Nov 2022 17:09:47 +0900 [thread overview]
Message-ID: <xmqq1qpwwwxg.fsf@gitster.g> (raw)
In-Reply-To: <pull.1424.git.1668972017089.gitgitgadget@gmail.com> (Sean Allred via GitGitGadget's message of "Sun, 20 Nov 2022 19:20:16 +0000")
"Sean Allred via GitGitGadget" <gitgitgadget@gmail.com> writes:
> From: Sean Allred <allred.sean@gmail.com>
>
> Provides the same benefits to scripts as exposing GIT_EDITOR, but
> allows distinguishing the 'sequence' editor from the 'core' editor.
>
> See also 44fcb4977cbae67f4698306ccfe982420ceebcbf.
Why should we ;-)?
If you explain why "git var GIT_SEQUENCE_EDITOR" is useful in a more
direct way, you do not even have to refer to a long hexadecimal string
which by itself does not mean anything to sane human beings.
The editor program used by Git when editing the sequencer "todo"
file is determined by examining a few environment variables and
also affected by configuration variables. Introduce "git var
GIT_SEQUENCE_EDITOR" that gives users an access to the final
result of the logic without having to know the exact detail.
This is very similar in spirit to 44fcb497 (Teach git var about
GIT_EDITOR, 2009-11-11) that introduced "git var GIT_EDITOR".
or something like that, perhaps?
> +GIT_SEQUENCE_EDITOR::
> + Text editor for use by Git sequencer commands. Like `GIT_EDITOR`,
Do our readers know what "Git sequencer commands" are? "rebase -i"
of course is the primary one, but "cherry-pick" and "revert" that
deals with multiple commits are technically "sequencer commands", as
they also use the sequencer machinery. But for them, the users do
not get a chance to edit the "todo" list with their sequence editor,
unlike "rebase -i".
I am wondering if it is easier to understand, without losing
technical correctness, to exactly name the command, without
pretending as if the sequence editor is used in situations wider
than where "rebase -i" is used, e.g.
The text editor program used to edit the 'todo' file while
running "git rebase -i".
or something.
> + the value is meant to be interpreted by the shell when it is used.
> + The order of preference is the `$GIT_SEQUENCE_EDITOR` environment
> + variable, then `sequence.editor` configuration, and then the value
> + of `git var GIT_EDITOR`.
OK.
> diff --git a/builtin/var.c b/builtin/var.c
> index 491db274292..9a2d31dc4aa 100644
> --- a/builtin/var.c
> +++ b/builtin/var.c
> @@ -19,6 +19,16 @@ static const char *editor(int flag)
> return pgm;
> }
>
> +static const char *sequence_editor(int flag)
> +{
> + const char *pgm = git_sequence_editor();
> +
> + if (!pgm && flag & IDENT_STRICT)
> + die("Terminal is dumb, but EDITOR unset");
I know this was copied from editor(), but the message does not make
much sense. It's not like the caller of read_var() is not prepared
to see a NULL returned, so letting it return NULL would make more
sense. Since the ancient past back when editor() function was
written, launch_editor() and the logic to die with "on dumb terminal
you must specify an EDITOR" have migrated to editor.c and there is
no strong reason to keep the corresponding die() even in editor()
function (I do not recommend removing it as part of this topic,
though), and adding a new one makes even less sense.
next prev parent reply other threads:[~2022-11-21 8:09 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-20 19:20 [PATCH] var: add GIT_SEQUENCE_EDITOR variable Sean Allred via GitGitGadget
2022-11-21 8:09 ` Junio C Hamano [this message]
2022-11-23 12:21 ` Sean Allred
2022-12-17 23:09 ` [PATCH v2] " Sean Allred via GitGitGadget
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq1qpwwwxg.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=allred.sean@gmail.com \
--cc=code@seanallred.com \
--cc=git@vger.kernel.org \
--cc=gitgitgadget@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).