* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-10-12 0:06 UTC (permalink / raw)
To: Christoph Anton Mitterer; +Cc: Junio C Hamano, git
In-Reply-To: <fbb3c2bf1c832f0f16cb913da6b862dd313359ef.camel@scientia.org>
On 2023-10-12 01:43, Christoph Anton Mitterer wrote:
> On Thu, 2023-10-12 at 01:29 +0200, Dragan Simic wrote:
>> I'm not sure what do you mean by the non-cleared remains being
>> chopped
>> off... Could you clarify that a bit, please?
>
> Well if I do say:
> $ reset
> $ git diff HEAD~10
>
> and from there scroll down a bit and then q to exit less (and the
> screen is not cleared), I see the output only so far as I've had
> previously scrolled down in less.
There's also scrollback in the terminal, which can be used to show more
of the contents that was displayed before exiting the pager.
> Everything that would have come after that is of course not visible.
> The place where I exited may be some "well defined" border, like the
> end of a commit... or anywhere it the middle of a patch (making the
> left over remains on the terminal perhaps even ambiguous).
If you didn't select some line or page to be displayed, by scrolling
within the pager, it obviously isn't going to be displayed, which is the
whole point of using a pager instead of "spitting" the whole contents
out at once.
Where and when you exit the pager is up to you only, and you can decide
what will be left on the screen at that point.
> What's worse, when (in less) I scroll down and up again, perhaps
> repeating several times, and then quit... I see (at least in my
> less/terminal combination) things twice and mangled up (i.e. when I
> scroll up the terminal (outside of less)).
That sounds like some issue with your terminal or terminal emulator,
which should be debugged and fixed separately. Such misbehavior isn't
supposed to happen at all.
> So AFAICS, not clearing the screen only works properly when never
> scrolling up again (in less).
It works just fine for me, for example. You're obviously having some
unrelated issues with your terminal emulator.
>> As I already mentioned above, everyone is free to configure the pager
>> behavior in any way they like.
>
> Sure :-)
>
>>
>> > > Exiting if less contents than one full screen was displayed (i.e.
>> > > "-
>> > > F")
>> > > is there to save people from the frustration of quitting a pager
>> > > that
>> > > actually wasn't needed to be executed.
>> >
>> > Same actually here, at least strictly speaking, ... though I (and
>> > probably everybody else?) would really hate it, if that was
>> > removed. ^^
>>
>> I'm afraid that I don't understand very well are you complaining
>> about
>> the presence of "-F" or not?
>
> No :-) As I've said, I like it that way and I and probably everyone
> else would be annoyed, if -F was not present.
>
> I just meant that strictly speaking the same reason why "S" was
> removed, could be applied to "F" as well.
I see. Actually, removing "-S" was a good decision, IMHO, because
chopping long lines isn't something that a sane set of defaults should
do. Many users would probably be confused with the need to use the
right arrow to see long lines in their entirety.
> It is - like -R - not necessary for less to work with git.
>
> But it is, of course, what virtually everyone will want in practise.
Well, "-R" is pretty much mandatory, because coloring the outputs has
become some kind of defacto standard, and it's very useful when viewing
diffs, for example.
>> Quite frankly, I can't stand scrolling in less(1) using the mouse
>> wheel,
>> but I do understand why some people like it.
>
> The main reason I want it is, that things don't get messy, when I
> forget being in less and mouse scroll. ;-)
>
>
> Thanks,
> Chris.
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-10-12 0:16 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Christoph Anton Mitterer, git
In-Reply-To: <20231012000416.GA520855@coredump.intra.peff.net>
On 2023-10-12 02:04, Jeff King wrote:
> On Wed, Oct 11, 2023 at 03:23:20PM -0700, Junio C Hamano wrote:
>
>> Christoph Anton Mitterer <calestyo@scientia.org> writes:
>>
>> > But I still don't get from that why X would be needed?
>> >
>> > My less manpage documents it as:
>> >> -X or --no‐init
>> >> Disables sending the termcap initialization and deinitialization
>> >> strings to the terminal. This is sometimes desirable if the
>> >> deinitialization string does something unnecessary, like clearing
>> >> the screen.
>> >
>> > Is it to avoid clearing the screen?
>>
>> I think that was the reason we added it back in 2005. In any case,
>> asking "why" is not a useful use of anybody's time, because it is
>> very unlikely to change in the official version we ship, and because
>> it is so easy for any individual who does not like it to drop by
>> exporting the $LESS environment variable.
>
> I agree it is probably not worth changing now, but I think the history
> here is a little interesting.
>
> Yes, I think "X" was added because less would clear the screen after
> exiting, and with "F" this meant you'd see nothing. Here's a thread
> from
> the same time period discussing it:
>
>
> https://lore.kernel.org/git/cc723f590610210623sbee2075i5f2fd441cceb84ae@mail.gmail.com/
>
> But I also think this was a pretty well-known annoyance with "less"
> back
> then.
>
> However, I can't seem to reproduce it now! Digging into the history and
> the changelog, this note is in "changes between less versions 487 and
> 530":
>
> Don't output terminal init sequence if using -F and file fits on one
> screen.
>
> So it seems like the problem has been fixed inside less for recent
> versions. And in theory we _could_ drop "-X" if it is causing problems.
> That version of less is ~5 years old. It does seem a little premature
> to
> assume everybody has it. And as you say, if there are people who really
> care about their LESS options, it is easy for them to override it.
Thanks for this detailed analysis!
It's important to keep in mind that removing "-X" and leaving "-F" would
introduce an inconsistency in the displaying of outputs, by having the
long outputs disappear after exiting the pager manually, and by having
the short outputs remain displayed after the pager exits on its own.
On the other hand, removing both "-X" and "-F" would make people even
more annoyed by requiring them to exit the pager manually even for short
outputs.
Quite frankly, "-FRX" is a just fine set of defaults.
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Christoph Anton Mitterer @ 2023-10-12 0:22 UTC (permalink / raw)
To: Dragan Simic; +Cc: Junio C Hamano, git
In-Reply-To: <ace230a469fabbbbceb38cc884a40b4c@manjaro.org>
On Thu, 2023-10-12 at 02:06 +0200, Dragan Simic wrote:
> There's also scrollback in the terminal, which can be used to show
> more
> of the contents that was displayed before exiting the pager.
Sure.
> > Everything that would have come after that is of course not
> > visible.
> > The place where I exited may be some "well defined" border, like
> > the
> > end of a commit... or anywhere it the middle of a patch (making the
> > left over remains on the terminal perhaps even ambiguous).
>
> If you didn't select some line or page to be displayed, by scrolling
> within the pager, it obviously isn't going to be displayed, which is
> the
> whole point of using a pager instead of "spitting" the whole contents
> out at once.
It's also clear that it's one point of a pager :-)
But that doesn't change that it's rather a user decision, whether or
not it makes sense to leave that, what's already been shown by the
pager, on the terminal after exiting the pager or not.
I don't think people always select the lines in the pager to some
reasonable border (e.g. end of a commit, end of a hunk, whatever).
So it's likely that after leaving the pager, the terminal's scrollback
buffer will contain something that is not complete and may thus be
ambiguous.
>
> That sounds like some issue with your terminal or terminal emulator,
> which should be debugged and fixed separately. Such misbehavior
> isn't
> supposed to happen at all.
Are you sure about that?
Well it happens at least in gnome-terminal, xterm and (KDE) konsole.
> I see. Actually, removing "-S" was a good decision, IMHO, because
> chopping long lines isn't something that a sane set of defaults
> should
> do. Many users would probably be confused with the need to use the
> right arrow to see long lines in their entirety.
Sure.
And having -F is IMO a good default (that virtually everyone would
want), too.
With respect to -X, I'm less sure whether it's that clear.
Cheers,
Chris.
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-10-12 0:31 UTC (permalink / raw)
To: Christoph Anton Mitterer; +Cc: Junio C Hamano, git
In-Reply-To: <8f3bec2752f4c2d3ebdd29d20910a4a94f75f608.camel@scientia.org>
On 2023-10-12 02:22, Christoph Anton Mitterer wrote:
> On Thu, 2023-10-12 at 02:06 +0200, Dragan Simic wrote:
>> There's also scrollback in the terminal, which can be used to show
>> more
>> of the contents that was displayed before exiting the pager.
>
> Sure.
>
>> > Everything that would have come after that is of course not
>> > visible.
>> > The place where I exited may be some "well defined" border, like
>> > the
>> > end of a commit... or anywhere it the middle of a patch (making the
>> > left over remains on the terminal perhaps even ambiguous).
>>
>> If you didn't select some line or page to be displayed, by scrolling
>> within the pager, it obviously isn't going to be displayed, which is
>> the
>> whole point of using a pager instead of "spitting" the whole contents
>> out at once.
>
> It's also clear that it's one point of a pager :-)
>
> But that doesn't change that it's rather a user decision, whether or
> not it makes sense to leave that, what's already been shown by the
> pager, on the terminal after exiting the pager or not.
>
> I don't think people always select the lines in the pager to some
> reasonable border (e.g. end of a commit, end of a hunk, whatever).
> So it's likely that after leaving the pager, the terminal's scrollback
> buffer will contain something that is not complete and may thus be
> ambiguous.
Makes sense, but please see also my other reply on the list. To sum it
up, we can have either the current behavior, the inconsistent behavior,
or an even more annoying behavior. I believe that the current behavior
is the best choice among these three options.
>> That sounds like some issue with your terminal or terminal emulator,
>> which should be debugged and fixed separately. Such misbehavior
>> isn't
>> supposed to happen at all.
>
> Are you sure about that?
>
> Well it happens at least in gnome-terminal, xterm and (KDE) konsole.
Yes, I'm sure, because I'd be fixing that already if that were the case
in my environment. :) I use Xfce and its default terminal emulator,
though, and I don't know what it's like in other desktop environments
and their terminal emulators.
>> I see. Actually, removing "-S" was a good decision, IMHO, because
>> chopping long lines isn't something that a sane set of defaults
>> should
>> do. Many users would probably be confused with the need to use the
>> right arrow to see long lines in their entirety.
>
> Sure.
>
> And having -F is IMO a good default (that virtually everyone would
> want), too.
>
> With respect to -X, I'm less sure whether it's that clear.
Please see my other response, which explains why having "-FX" is
actually a good thing.
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Junio C Hamano @ 2023-10-12 1:39 UTC (permalink / raw)
To: Jeff King; +Cc: Christoph Anton Mitterer, git
In-Reply-To: <20231012000416.GA520855@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
> ... Digging into the history and
> the changelog, this note is in "changes between less versions 487 and
> 530":
>
> Don't output terminal init sequence if using -F and file fits on one
> screen.
;-)
That is really a good one to dig out. So in short, X was needed
because we wanted to use F, and we could drop it if everybody is
using recent versions of less, but the default to use FX at the same
time gives us the same behaviour between both newer and older
versions of less.
> ... And as you say, if there are people who really
> care about their LESS options, it is easy for them to override it.
Yup. I really like the discovery of that changelog entry.
Thanks.
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Christoph Anton Mitterer @ 2023-10-12 1:39 UTC (permalink / raw)
To: Dragan Simic; +Cc: Junio C Hamano, git
In-Reply-To: <23cc509bfb433e19c7683c97314e4ac8@manjaro.org>
On Thu, 2023-10-12 at 02:31 +0200, Dragan Simic wrote:
>
> Makes sense, but please see also my other reply on the list. To sum
> it
> up, we can have either the current behavior, the inconsistent
> behavior,
> or an even more annoying behavior. I believe that the current
> behavior
> is the best choice among these three options.
Well as I've said... I don't demand that it's changed, but I simply
think it's a wrong assumption that it's in any way better or worse.
Leaving back partial output, or when scrolling up&down completely
messed up output, is surely not per se more annoying or a bigger
problem than leaving back no output at all in one case (when it doesn't
fit on one screen) or leaving back output (when it fits).
> Yes, I'm sure, because I'd be fixing that already if that were the
> case
> in my environment. :) I use Xfce and its default terminal emulator,
> though, and I don't know what it's like in other desktop environments
> and their terminal emulators.
I just tried it with xfce4-terminal 1.1.0 (which AFAICS is the most
recent version) in Debian, and unless they break anything with custom
patches, or you distro fixes anything with custom patches... I'd say
you must suffer from the same issue and probably just try something
different.
Since Debian's less is pretty outdated, I've even compiled a quite
recent less 643 (there's not even a tarball yet for 644, only a git
tag).
A made a screen recording... it's not 8K ;-) but I guess you can see
what I do:
https://youtu.be/KMs3sLk9nXY
Cheers,
Chris.
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Christoph Anton Mitterer @ 2023-10-12 3:54 UTC (permalink / raw)
To: Jeff King, Junio C Hamano; +Cc: git
In-Reply-To: <20231012000416.GA520855@coredump.intra.peff.net>
Hey.
Just noted that the popular bat utility apparently also uses -X to make
-F work (but also mention that this break scrolling).
But it seem they have a check, an if less is version 530 or newer they
don't set -X.
https://github.com/sharkdp/bat#using-a-different-pager
Could be a way to go for git.
Cheers,
Chris.
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-10-12 5:30 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, Christoph Anton Mitterer, git
In-Reply-To: <xmqqh6mwipqi.fsf@gitster.g>
On 2023-10-12 03:39, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
>> ... Digging into the history and
>> the changelog, this note is in "changes between less versions 487 and
>> 530":
>>
>> Don't output terminal init sequence if using -F and file fits on one
>> screen.
>
> ;-)
>
> That is really a good one to dig out. So in short, X was needed
> because we wanted to use F, and we could drop it if everybody is
> using recent versions of less, but the default to use FX at the same
> time gives us the same behaviour between both newer and older
> versions of less.
Please note that dropping "-X" and leaving "-F" would actually introduce
the inconsistency that I already mentioned. To reiterate, short outputs
would then remain displayed on screen, while long outputs would
disappear after exiting less(1). I don't think that's the desired
behavior.
>> ... And as you say, if there are people who really
>> care about their LESS options, it is easy for them to override it.
>
> Yup. I really like the discovery of that changelog entry.
>
> Thanks.
^ permalink raw reply
* Re: [PATCH 1/1] branch.c: ammend error messages for die()
From: Isoken Ibizugbe @ 2023-10-12 5:31 UTC (permalink / raw)
To: Rubén Justo; +Cc: git, Junio C Hamano
In-Reply-To: <06bc7b39-ed70-460f-b6f1-47a0c94c0538@gmail.com>
On Wed, Oct 11, 2023 at 7:05 PM Rubén Justo <rjusto@gmail.com> wrote:
>
> On 11-oct-2023 16:24:20, Isoken June Ibizugbe wrote:
>
> Hi Isoken,
>
> > Signed-off-by: Isoken June Ibizugbe <isokenjune@gmail.com>
> > ---
> > builtin/branch.c | 38 +++++++++++++++++++-------------------
> > 1 file changed, 19 insertions(+), 19 deletions(-)
> >
> > diff --git a/builtin/branch.c b/builtin/branch.c
> > index 2ec190b14a..a756543d64 100644
> > --- a/builtin/branch.c
> > +++ b/builtin/branch.c
> > @@ -518,11 +518,11 @@ static void reject_rebase_or_bisect_branch(struct worktree **worktrees,
> > continue;
> >
> > if (is_worktree_being_rebased(wt, target))
> > - die(_("Branch %s is being rebased at %s"),
> > + die(_("branch %s is being rebased at %s"),
>
> OK.
>
> > target, wt->path);
> >
> > if (is_worktree_being_bisected(wt, target))
> > - die(_("Branch %s is being bisected at %s"),
> > + die(_("branch %s is being bisected at %s"),
>
> OK.
>
> > target, wt->path);
> > }
> > }
> > @@ -578,7 +578,7 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
> > if (ref_exists(oldref.buf))
> > recovery = 1;
> > else
> > - die(_("Invalid branch name: '%s'"), oldname);
> > + die(_("invalid branch name: '%s'"), oldname);
>
> OK.
>
> > }
> >
> > for (int i = 0; worktrees[i]; i++) {
> > @@ -594,9 +594,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
> >
> > if ((copy || !(oldref_usage & IS_HEAD)) && !ref_exists(oldref.buf)) {
> > if (oldref_usage & IS_HEAD)
> > - die(_("No commit on branch '%s' yet."), oldname);
> > + die(_("no commit on branch '%s' yet"), oldname);
> > else
> > - die(_("No branch named '%s'."), oldname);
> > + die(_("no branch named '%s'"), oldname);
>
> OK. Both.
>
> > }
> >
> > /*
> > @@ -624,9 +624,9 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
> >
> > if (!copy && !(oldref_usage & IS_ORPHAN) &&
> > rename_ref(oldref.buf, newref.buf, logmsg.buf))
> > - die(_("Branch rename failed"));
> > + die(_("branch rename failed"));
> > if (copy && copy_existing_ref(oldref.buf, newref.buf, logmsg.buf))
> > - die(_("Branch copy failed"));
> > + die(_("branch copy failed"));
>
> Ditto
>
> >
> > if (recovery) {
> > if (copy)
> > @@ -640,16 +640,16 @@ static void copy_or_rename_branch(const char *oldname, const char *newname, int
> > if (!copy && (oldref_usage & IS_HEAD) &&
> > replace_each_worktree_head_symref(worktrees, oldref.buf, newref.buf,
> > logmsg.buf))
> > - die(_("Branch renamed to %s, but HEAD is not updated!"), newname);
> > + die(_("branch renamed to %s, but HEAD is not updated!"), newname);
>
> IMO we can go further here and also remove that final "!". But it's OK
> the way you have done it.
>
> >
> > strbuf_release(&logmsg);
> >
> > strbuf_addf(&oldsection, "branch.%s", interpreted_oldname);
> > strbuf_addf(&newsection, "branch.%s", interpreted_newname);
> > if (!copy && git_config_rename_section(oldsection.buf, newsection.buf) < 0)
> > - die(_("Branch is renamed, but update of config-file failed"));
> > + die(_("branch is renamed, but update of config-file failed"));
> > if (copy && strcmp(interpreted_oldname, interpreted_newname) && git_config_copy_section(oldsection.buf, newsection.buf) < 0)
> > - die(_("Branch is copied, but update of config-file failed"));
> > + die(_("branch is copied, but update of config-file failed"));
>
> OK, both.
>
> > strbuf_release(&oldref);
> > strbuf_release(&newref);
> > strbuf_release(&oldsection);
> > @@ -773,7 +773,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> >
> > head = resolve_refdup("HEAD", 0, &head_oid, NULL);
> > if (!head)
> > - die(_("Failed to resolve HEAD as a valid ref."));
> > + die(_("failed to resolve HEAD as a valid ref"));
>
> OK.
>
> > if (!strcmp(head, "HEAD"))
> > filter.detached = 1;
> > else if (!skip_prefix(head, "refs/heads/", &head))
> > @@ -866,7 +866,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> >
> > if (!argc) {
> > if (filter.detached)
> > - die(_("Cannot give description to detached HEAD"));
> > + die(_("cannot give description to detached HEAD"));
>
> OK.
>
> > branch_name = head;
> > } else if (argc == 1) {
> > strbuf_branchname(&buf, argv[0], INTERPRET_BRANCH_LOCAL);
> > @@ -892,7 +892,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> > if (!argc)
> > die(_("branch name required"));
> > else if ((argc == 1) && filter.detached)
> > - die(copy? _("cannot copy the current branch while not on any.")
> > + die(copy? _("cannot copy the current branch while not on any")
> > : _("cannot rename the current branch while not on any."));
>
> OK. But I think you want to modify also the second message, to remove
> its full stop as well.
>
> > else if (argc == 1)
> > copy_or_rename_branch(head, argv[0], copy, copy + rename > 1);
> > @@ -916,14 +916,14 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> > if (!branch) {
> > if (!argc || !strcmp(argv[0], "HEAD"))
> > die(_("could not set upstream of HEAD to %s when "
> > - "it does not point to any branch."),
> > + "it does not point to any branch"),
>
> OK.
>
> > new_upstream);
> > die(_("no such branch '%s'"), argv[0]);
> > }
> >
> > if (!ref_exists(branch->refname)) {
> > if (!argc || branch_checked_out(branch->refname))
> > - die(_("No commit on branch '%s' yet."), branch->name);
> > + die(_("no commit on branch '%s' yet"), branch->name);
>
> OK.
>
> > die(_("branch '%s' does not exist"), branch->name);
> > }
> >
> > @@ -946,12 +946,12 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> > if (!branch) {
> > if (!argc || !strcmp(argv[0], "HEAD"))
> > die(_("could not unset upstream of HEAD when "
> > - "it does not point to any branch."));
> > + "it does not point to any branch"));
> > die(_("no such branch '%s'"), argv[0]);
> > }
> >
> > if (!branch_has_merge_config(branch))
> > - die(_("Branch '%s' has no upstream information"), branch->name);
> > + die(_("branch '%s' has no upstream information"), branch->name);
>
> OK, both.
>
> >
> > strbuf_reset(&buf);
> > strbuf_addf(&buf, "branch.%s.remote", branch->name);
> > @@ -965,11 +965,11 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> > const char *start_name = argc == 2 ? argv[1] : head;
> >
> > if (filter.kind != FILTER_REFS_BRANCHES)
> > - die(_("The -a, and -r, options to 'git branch' do not take a branch name.\n"
> > + die(_("the -a, and -r, options to 'git branch' do not take a branch name\n"
> > "Did you mean to use: -a|-r --list <pattern>?"));
>
> Good; the full stop removed and here that question mark is valuable. And ...
>
> >
> > if (track == BRANCH_TRACK_OVERRIDE)
> > - die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
> > + die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead"));
>
> also good. But since we're here, maybe we can break this long message, remove
> the first full stop and leave the second part of the message as is, as a
> suggestion. Like we do in the previous message, above.
>
> For your consideration, I mean something like:
>
> --- a/builtin/branch.c
> +++ b/builtin/branch.c
> @@ -969,7 +969,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
> "Did you mean to use: -a|-r --list <pattern>?"));
>
> if (track == BRANCH_TRACK_OVERRIDE)
> - die(_("the '--set-upstream' option is no longer supported. Please use '--track' or '--set-upstream-to' instead."));
> + die(_("the '--set-upstream' option is no longer supported\n"
> + "Please use '--track' or '--set-upstream-to' instead."));
>
>
> >
> > if (recurse_submodules) {
> > create_branches_recursively(the_repository, branch_name,
> > --
> > 2.42.0.325.g3a06386e31
> >
>
> One final comment, leaving aside my suggestions; this changes break
> some tests that you need to adjust. Try:
>
> $ make && (cd t; ./t3200-branch.sh -vi)
>
> Or, as Junio has already suggested in another message:
>
> $ make test
>
> I have some unfinished work that you might find useful:
>
> https://lore.kernel.org/git/eb3c689e-efeb-4468-a10f-dd32bc0ee37b@gmail.com/
Thanks for the review. I'll make the requested changes and provide an
update. I'll also make sure to run the tests you suggested and make
the necessary changes.
>
> Thank you for working on this.
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-10-12 5:46 UTC (permalink / raw)
To: Christoph Anton Mitterer; +Cc: Junio C Hamano, git
In-Reply-To: <2f3ef5568ed19ac5bdcd23f84ddfb13dc6901043.camel@scientia.org>
On 2023-10-12 03:39, Christoph Anton Mitterer wrote:
> On Thu, 2023-10-12 at 02:31 +0200, Dragan Simic wrote:
>>
>> Makes sense, but please see also my other reply on the list. To sum
>> it
>> up, we can have either the current behavior, the inconsistent
>> behavior,
>> or an even more annoying behavior. I believe that the current
>> behavior
>> is the best choice among these three options.
>
> Well as I've said... I don't demand that it's changed, but I simply
> think it's a wrong assumption that it's in any way better or worse.
>
> Leaving back partial output, or when scrolling up&down completely
> messed up output, is surely not per se more annoying or a bigger
> problem than leaving back no output at all in one case (when it doesn't
> fit on one screen) or leaving back output (when it fits).
Let me repeat that the messed up output you're experiencing isn't normal
and has nothing to do with the arguments passed to less(1). That's a
separate issue of the terminal emulator(s) you're using, or in issue of
your specific environment, and should be debugged and addressed as a
separate issue.
To me, having inconsistent displaying of the short and long outputs is
simply not acceptable.
>> Yes, I'm sure, because I'd be fixing that already if that were the
>> case
>> in my environment. :) I use Xfce and its default terminal emulator,
>> though, and I don't know what it's like in other desktop environments
>> and their terminal emulators.
>
> I just tried it with xfce4-terminal 1.1.0 (which AFAICS is the most
> recent version) in Debian, and unless they break anything with custom
> patches, or you distro fixes anything with custom patches... I'd say
> you must suffer from the same issue and probably just try something
> different.
Let me repeat that I don't see those issues, and actually, IIRC, have
never seen them in my 25+ years of using various Linux distributions.
If I've ever seen that, it would've already motivated me to have it
debugged and fixed.
Perhaps something is wrong with your specific environment, because I see
no other reason for this issue.
> Since Debian's less is pretty outdated, I've even compiled a quite
> recent less 643 (there's not even a tarball yet for 644, only a git
> tag).
>
> A made a screen recording... it's not 8K ;-) but I guess you can see
> what I do:
> https://youtu.be/KMs3sLk9nXY
For some reason, I can see it with 360p as the highest available
resolution, so I really can't read what's displayed on the recorded
screen. Strange. Could you, please, upload the video in higher
resolution, perhaps to a file sharing service such as
https://easyupload.io/ ?
^ permalink raw reply
* Re: why does git set X in LESS env var?
From: Dragan Simic @ 2023-10-12 5:57 UTC (permalink / raw)
To: Christoph Anton Mitterer; +Cc: Jeff King, Junio C Hamano, git
In-Reply-To: <07bf5744c7d123635740c62a940999e089339fa4.camel@scientia.org>
On 2023-10-12 05:54, Christoph Anton Mitterer wrote:
> Hey.
>
> Just noted that the popular bat utility apparently also uses -X to make
> -F work (but also mention that this break scrolling).
>
> But it seem they have a check, an if less is version 530 or newer they
> don't set -X.
>
> https://github.com/sharkdp/bat#using-a-different-pager
>
> Could be a way to go for git.
Let me repeat that the described scrolling-related issues are
misleading. I use this, configured through $BAT_PAGER, and have never
experienced such issues in a few years of using bat(1):
├─bash
│ └─bat -n --tabs 8 --wrap never message.txt
│ └─less -R -F -X
^ permalink raw reply
* Wow
From: Edward Zmewski jr @ 2023-10-12 6:27 UTC (permalink / raw)
To: git
Not exactly sure who giving me these directions, but I’m very good at what I do to, all do respect, I tried just don’t register, I can use computers, but you guys are brilliant and I respect it, but I’ll try , but we wouldn’t be having a conversation if I was that good at programming, seriously, my apologies for not thinking to look out for you guys, that’s my mistake, listen I will help you guys if you can help me, thank you
,
^ permalink raw reply
* [Outreachy]Introduction and Problem while installing Git
From: Dorcas Litunya @ 2023-10-12 6:57 UTC (permalink / raw)
To: git
Hello everyone,
My name is Dorcas Litunya. I am excited to contribute to the git
community, I am a first time contributor through the Outreachy program.
I am excited to learn and grow through this project. I am currently
installing Git and I have been faced with this error once I run the make
command:
In file included from http.c:2:
git-curl-compat.h:3:10: fatal error: curl/curl.h: No such file or directory
3 | #include <curl/curl.h>
| ^~~~~~~~~~~~~
compilation terminated.
make: *** [Makefile:2718: http.o] Error 1
Any assistance on what the possible problem could be will be highly
appreciated.
Kind regards,
Dorcas
^ permalink raw reply
* Re: [Outreachy]Introduction and Problem while installing Git
From: Benson Muite @ 2023-10-12 7:28 UTC (permalink / raw)
To: Dorcas Litunya, git
In-Reply-To: <ZSeYzdx07Cj67lR4@dorcaslitunya-virtual-machine>
On 10/12/23 09:57, Dorcas Litunya wrote:
> Hello everyone,
> My name is Dorcas Litunya. I am excited to contribute to the git
> community, I am a first time contributor through the Outreachy program.
> I am excited to learn and grow through this project. I am currently
> installing Git and I have been faced with this error once I run the make
> command:
> In file included from http.c:2:
> git-curl-compat.h:3:10: fatal error: curl/curl.h: No such file or directory
> 3 | #include <curl/curl.h>
> | ^~~~~~~~~~~~~
You will need to have curl libraries and development headers.
https://curl.se/libcurl/
You maybe able to get these from a package manager, for example on Ubuntu
sudo apt-get install curl-dev
Fedora
sudo dnf install libcurl-devel
^ permalink raw reply
* Re: [Outreachy]Introduction and Problem while installing Git
From: Kristoffer Haugsbakk @ 2023-10-12 7:43 UTC (permalink / raw)
To: Benson Muite; +Cc: Dorcas Litunya, git
In-Reply-To: <4c5fef38-a671-dd6b-4b10-a531e1ae254a@emailplus.org>
On Thu, Oct 12, 2023, at 09:28, Benson Muite wrote:
> You maybe able to get these from a package manager, for example on Ubuntu
> sudo apt-get install curl-dev
This didn't work for me on Ubuntu 22.04. I tried the OpenSSL variant which
worked.
sudo apt-get install libcurl4-openssl-dev
Now I can drop `NO_CURL=true`.
Thanks :)
--
Kristoffer Haugsbakk
^ permalink raw reply
* Re: [Outreachy]Introduction and Problem while installing Git
From: Dorcas Litunya @ 2023-10-12 7:52 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: benson_muite, git
In-Reply-To: <724ff75e-1557-4d87-85a6-a3a58d41af1f@app.fastmail.com>
On Thu, Oct 12, 2023 at 09:43:55AM +0200, Kristoffer Haugsbakk wrote:
> On Thu, Oct 12, 2023, at 09:28, Benson Muite wrote:
> > You maybe able to get these from a package manager, for example on Ubuntu
> > sudo apt-get install curl-dev
>
> This didn't work for me on Ubuntu 22.04. I tried the OpenSSL variant which
> worked.
>
Me too
> sudo apt-get install libcurl4-openssl-dev
>
This package has worked for me on Ubuntu 22.04
Thanks for the help!
> Now I can drop `NO_CURL=true`.
>
> Thanks :)
>
> --
> Kristoffer Haugsbakk
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Josh Triplett @ 2023-10-12 8:47 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kristoffer Haugsbakk, Sebastian Thiel, git
In-Reply-To: <xmqqo7h6tnib.fsf@gitster.g>
On Tue, Oct 10, 2023 at 10:07:08AM -0700, Junio C Hamano wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
>
> > While I'd love for it to default to that and require an extra option to
> > clean away precious files, I'd expect that that would break people's
> > workflows and finger memory. If someone expects `git clean -x -d -f` to
> > clean away everything, including `.config`, and then it leaves some
> > files in place, that seems likely to cause problems. (Leaving aside that
> > it might break scripted workflows.)
>
> I thought the point of introducing the new "precious" class of
> paths, in addition to the current "tracked", "ignored, untracked,
> and expendable", "not ignored and untracked", is so that people can
> do "git clean -x -d -f" and expect the ".config" that is marked as
> "precious" to stay. Before their Git learned the precious class, if
> they marked ".config" as "ignored, untracked, and expendable", then
> such an invocation of "clean" would have removed it, but if they add
> it to the new "precious" class, their expectation ought to be that
> precious ones are not removed, no? Otherwise I am not quite sure
> what the point of adding such a new protection is.
I'd expect a lot of projects to move things *from* the current "ignored"
state to "precious", once "precious" exists. Linux `.config`, for
instance.
That said, I do agree that the ideal behavior is for clean to preserve
precious files by default, and require an extra option to remove
precious files. If you think that doesn't have backwards-compatibility
considerations, then it certainly seems much easier to jump directly to
that behavior.
- Josh Triplett
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Josh Triplett @ 2023-10-12 9:04 UTC (permalink / raw)
To: Kristoffer Haugsbakk; +Cc: Sebastian Thiel, git
In-Reply-To: <25b25127-aa10-4179-bc02-065fe12d01ef@app.fastmail.com>
On Tue, Oct 10, 2023 at 09:10:34PM +0200, Kristoffer Haugsbakk wrote:
> Hi Josh
>
> On Tue, Oct 10, 2023, at 16:10, Josh Triplett wrote:
> > > [snip]
> >
> > While I'd love for it to default to that and require an extra option to
> > clean away precious files, I'd expect that that would break people's
> > workflows and finger memory. If someone expects `git clean -x -d -f` to
> > clean away everything, including `.config`, and then it leaves some
> > files in place, that seems likely to cause problems. (Leaving aside that
> > it might break scripted workflows.)
> >
> > It seems safer to keep the existing behavior for existing options, and
> > add a new option for "remove everything except precious files".
>
> What's a scenario where it breaks? I'm guessing:
>
> 1. Someone clones a project
> 2. That project has precious files marked via `.gitattributes`
> 3. They later do a `clean`
> 4. The precious files are left alone even though they expected them to be
> deleted; they don't check what `clean` did (it deletes everything
> untracked (they expect) so nothing to check)
> 5. This hurts them somehow
The scenario I had in mind was:
- Project has ignored files; git doesn't have a concept of "precious"
- Users expect that `git clean -x -d -f` deletes everything that isn't
part of the latest commit.
- Git introduces the concept of "precious"
- Project adopts "precious" and marks some of its ignored files as
"precious" instead
- Users' finger-macros around `git clean` stop cleaning up files they
expected to be cleaned.
That said, given Junio's response I'm no longer concerned about this
scenario.
^ permalink raw reply
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Karthik Nayak @ 2023-10-12 10:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Patrick Steinhardt, git
In-Reply-To: <xmqqbkd5nlq0.fsf@gitster.g>
On Wed, Oct 11, 2023 at 6:54 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Karthik Nayak <karthik.188@gmail.com> writes:
>
> > Seems like this is because of commit-graph being enabled, I think
> > the best thing to do here would be to disable the commit graph of
> > these tests.
>
> If the CI uncovered that the new code is broken and does not work
> with commit-graph, wouldn't the above a totally wrong approach to
> correct it? If the updated logic cannot work correctly when
> commit-graph covers the history you intentionally break, shouldn't
> the code, when the feature that is incompatible with commit-graph is
> triggered, disable the commit-graph? I am assuming that the new
> feature is meant to be used to recover from a corrupt repository,
> and if it does not work well when commit-graph knows (now stale
> after repository corruption) more about the objects that are corrupt
> in the object store, we do want to disable commit-graph. After all,
> commit-graph is a secondary information that is supposed to be
> recoverable from the primary data that is what is in the object
> store. Disabling commit-graph in the test means you are telling the
> end-users "do not use commit-graph if you want to use this feature",
> which sounds like a wrong thing to do.
I agree with what you're saying. Disabling writing the commit-graph for
only the test doesn't serve the real usage. To ensure that the feature should
work with corrupt repositories with stale commit-graph, I'm thinking of
disabling the commit-graph whenever the `--missing` option is used. The
commit graph already provides an API for this, so it should be simple to do.
Thanks!
^ permalink raw reply
* Re: [RFC] Define "precious" attribute and support it in `git clean`
From: Sebastian Thiel @ 2023-10-12 10:55 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Josh Triplett, Kristoffer Haugsbakk
In-Reply-To: <xmqqttqytnqb.fsf@gitster.g>
I liked the idea too see precious files as sub-class of ignored files, and
investigated possibilities on how to achieve that while keeping the overall
effort low and remove any potential for backwards-incompatibility as well.
Currently, `.gitignore` files only contain one pattern per line, which
optionally may be prefixed with `!` to negate it. This can be escaped with `\!`
- and that's it.
Parsing patterns that way makes for simple parsing without a need for
quoting.
### What about a `$` syntax in `.gitignore` files?
I looked into adding a new prefix, `$` to indicate the following path is
precious or… valuable. It can be escaped with `\$` just like `\!`.
Doing so has the advantage that older `git` versions simply take the
declaration as literal and would now exclude `$.config`, for example, whereas
newer `git` versions will consider them precious.
There is some potential for accidentally excluding files that previously
were untracked with older versions of git, but I'd think chances are low.
#### Example: Linux kernel
`.config` is ignored via `.gitignore:
.*
*Unfortunately*, users can't just add a local `.git/info/exclude` file with
`$.config` in it and expect `.config` to be considered precious as the pattern
search order will search this last as it's part of the exclude-globals. The
same is true for per-user git-ignore files. This means that any git would
have the `.*` pattern match before the `$.config` pattern, and stop right there
concluding that it's expendable, instead of precious. This is how users can
expect `.gitignore` files to work, and this is how `!negations` work as well -
the negation has to come after the actual exclusion to be effective.
Thus, to make this work, projects that ship the `.gitignore` files would *have
to add patterns* that make certain files precious.
Alternatively, users have to specify gitignore-overrides on the command-line,
but not all commands (if any except for `git check-ignore`) support that.
In the case of `git clean` one can already pass `--exclude=pattern`, but if
that's needed one doesn't need syntax for precious files in the first place.
**This makes the $precious syntax useful only for projects who chose to opt in,
but makes overrides for users near impossible**.
Such opted-in projects would produce `.gitignore` files like these:
.*
$.config
Note that due to the way ignore patterns are searched, the following would
consider `.config` trackable, not precious:
.*
$.config
!.config
It's up the maintainer of the repository to configure their .gitignore files
correctly, so nothing new either.
#### Benefits
* simple implementation, fast
* backwards compatible
#### Disadvantages
* cannot easily be overridden by the user as part of their local settings.
* needs repository-buy-in to be useful
* $file could clash with the file '$file' and cause older git to ignore it
### What about a `precious` attribute?
The search of `.gitattributes` works differently which makes it possible for
users to set attributes on any file or folder easily using their local files.
Using attributes has the added benefit of being extensible as one can start out
with:
```gitattributes
.config precious
```
and optionally continue with…
```gitattributes
.config precious=input
kernel precious=output
```
…to further classify kinds of precious files, probably for their personal use.
Please note that currently pathspecs can't be used to filter by attribute
for files that are igonred and untracked or I couldn't figure out how.
That even makes sense as it wasn't considered a use-case yet.
#### Benefits
* backwards compatible
* easily extendable with 'tags' or sub-classes of precious files using the
assignment syntax.
* overridable with user's local files
#### Disadvantages
* any 'exclude' query now also needs a .gitattribute query to support precious
files (and it's not easy to optimize unless there is a flag to turn precious
file support on or off)
* `precious` might be in use by some repos which now gains a possibly different
meaning in `git` as well.
### Conclusion
Weighing advantages and disadvantages of both approaches makes me prefer the
`.gitignore` extension. The `.gitattributes` version of it *could* also be
implemented on top of it at a later date. However, it should be gated behind a
configuration flag so users who need it as they want local overrides
can opt-in. Then they also pay for the feature which for most repositories
won't be an issue in the first place.
All this seems a bit too good to be true, and I hope you can show where
it wouldn't work or which dangers or possible issues haven't been
mentioned yet.
^ permalink raw reply
* Re: [PATCH 0/3] rev-list: add support for commits in `--missing`
From: Patrick Steinhardt @ 2023-10-12 11:04 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Junio C Hamano, git
In-Reply-To: <CAOLa=ZQxNX4oGtqrgLyKenC_D8M=9q0sFJVmo4fyjSPtgw315Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2669 bytes --]
On Thu, Oct 12, 2023 at 12:44:27PM +0200, Karthik Nayak wrote:
> On Wed, Oct 11, 2023 at 6:54 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Karthik Nayak <karthik.188@gmail.com> writes:
> >
> > > Seems like this is because of commit-graph being enabled, I think
> > > the best thing to do here would be to disable the commit graph of
> > > these tests.
> >
> > If the CI uncovered that the new code is broken and does not work
> > with commit-graph, wouldn't the above a totally wrong approach to
> > correct it? If the updated logic cannot work correctly when
> > commit-graph covers the history you intentionally break, shouldn't
> > the code, when the feature that is incompatible with commit-graph is
> > triggered, disable the commit-graph? I am assuming that the new
> > feature is meant to be used to recover from a corrupt repository,
> > and if it does not work well when commit-graph knows (now stale
> > after repository corruption) more about the objects that are corrupt
> > in the object store, we do want to disable commit-graph. After all,
> > commit-graph is a secondary information that is supposed to be
> > recoverable from the primary data that is what is in the object
> > store. Disabling commit-graph in the test means you are telling the
> > end-users "do not use commit-graph if you want to use this feature",
> > which sounds like a wrong thing to do.
>
> I agree with what you're saying. Disabling writing the commit-graph for
> only the test doesn't serve the real usage. To ensure that the feature should
> work with corrupt repositories with stale commit-graph, I'm thinking of
> disabling the commit-graph whenever the `--missing` option is used. The
> commit graph already provides an API for this, so it should be simple to do.
>
> Thanks!
Wouldn't this have the potential to significantly regress performance
for all those preexisting users of the `--missing` option? The commit
graph is quite an important optimization nowadays, and especially in
commands where we potentially walk a lot of commits (like we may do
here) it can result in queries that are orders of magnitudes faster.
If we were introducing a new option then I think this would be an
acceptable tradeoff as we could still fix the performance issue in
another iteration. But we don't and instead aim to change the meaning of
`--missing` which may already have users out there. Seen in that light
it does not seem sensible to me to just disable the graph for them.
Did you figure out what exactly the root cause of this functional
regression is? If so, can we address that root cause instead?
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH] send-email: add --compose-cover option
From: Ben Dooks @ 2023-10-12 11:27 UTC (permalink / raw)
To: git, gitster; +Cc: Ben Dooks
Adding an option to automatically compose a cover letter would be
helpful to put the whole process of sending an series with a cover
into one command.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
Documentation/git-send-email.txt | 5 +++++
git-send-email.perl | 11 ++++++++++-
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 41cd8cb424..f299732867 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -78,6 +78,11 @@ Missing From or In-Reply-To headers will be prompted for.
+
See the CONFIGURATION section for `sendemail.multiEdit`.
+--compose-cover::
+ Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
+ to edit a cover letter generated by passing --cover-letter to
+ git-send-email before invoking the editor.
+
--from=<address>::
Specify the sender of the emails. If not specified on the command line,
the value of the `sendemail.from` configuration option is used. If
diff --git a/git-send-email.perl b/git-send-email.perl
index 5861e99a6e..debec088f6 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -54,6 +54,7 @@ sub usage {
--in-reply-to <str> * Email "In-Reply-To:"
--[no-]xmailer * Add "X-Mailer:" header (default).
--[no-]annotate * Review each patch that will be sent in an editor.
+ --compose-cover * Open an editor on format-patch --cover-letter
--compose * Open an editor for introduction.
--compose-encoding <str> * Encoding to assume for introduction.
--8bit-encoding <str> * Encoding to assume 8bit mails if undeclared
@@ -199,7 +200,7 @@ sub format_2822_time {
# Variables we fill in automatically, or via prompting:
my (@to,@cc,@xh,$envelope_sender,
$initial_in_reply_to,$reply_to,$initial_subject,@files,
- $author,$sender,$smtp_authpass,$annotate,$compose,$time);
+ $author,$sender,$smtp_authpass,$annotate,$compose_cover,$compose,$time);
# Things we either get from config, *or* are overridden on the
# command-line.
my ($no_cc, $no_to, $no_bcc, $no_identity);
@@ -512,6 +513,7 @@ sub config_regexp {
"no-smtp-auth" => sub {$smtp_auth = 'none'},
"annotate!" => \$annotate,
"no-annotate" => sub {$annotate = 0},
+ "compose-cover" => \$compose_cover,
"compose" => \$compose,
"quiet" => \$quiet,
"cc-cmd=s" => \$cc_cmd,
@@ -782,6 +784,9 @@ sub is_format_patch_arg {
die __("Cannot run git format-patch from outside a repository\n")
unless $repo;
require File::Temp;
+ if ($compose_cover) {
+ push @rev_list_opts, "--cover-letter";
+ }
push @files, $repo->command('format-patch', '-o', File::Temp::tempdir(CLEANUP => 1), @rev_list_opts);
}
@@ -854,6 +859,8 @@ sub get_patch_subject {
if ($annotate) {
do_edit($compose_filename, @files);
+ } elsif ($compose_cover) {
+ do_edit($files[0]);
} else {
do_edit($compose_filename);
}
@@ -927,6 +934,8 @@ sub get_patch_subject {
} elsif ($annotate) {
do_edit(@files);
+} elsif ($compose_cover) {
+ do_edit($files[0]);
}
sub term {
--
2.42.0
^ permalink raw reply related
* How to combine multiple commit diffs?
From: ZheNing Hu @ 2023-10-12 12:00 UTC (permalink / raw)
To: Git List, Junio C Hamano
Hi everyone,
Our company wants to design a "small-batch" code review feature.
Simply put, this "small-batch" means being able to treat multiple
related commits within a MergeRequest as an independent "small" code
review.
Let me give you an example: We have five commits: A1, B, A2, C, A3.
Among them, A1, A2, and A3 are multiple commits for the same feature.
So when the user selects these commits, the page will return a
"combine diff" that combines them together.
A1 B A2 A3 C
*--------*----*-----*-------* (branch)
\ A1' \ A2' \ A3'
*------------*------*------- (small branch code review)
This may seem similar to cherry-picking a few commits from a pile of
commits, but in fact, we do not expect to actually perform
cherry-picking.
Do you have any suggestions on how we can merge a few commits together
and display the diff? The only reference we have is the non-open
source platform, JetBrains Space CodeReview, they support selecting
multiple commits for CodeReview. [1], .
[1]: https://www.jetbrains.com/help/space/review-code.html#code-review-example
Thanks,
--
ZheNing Hu
^ permalink raw reply
* Bug: Git grep -f reads the filename relative to the repository root
From: Erik Cervin Edin @ 2023-10-12 12:38 UTC (permalink / raw)
To: Git Mailing List
Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.
What did you do before the bug happened? (Steps to reproduce your issue)
In the Git repository, I ran
echo tig > pattern-file &&
echo git > xdiff/pattern-file &&
cd xdfiff &&
git grep -f pattern-file
What did you expect to happen? (Expected behavior)
Git grep -f to read the pattern-file, in the xdiff directory and
search for lines matching `git` in the xdiff directory.
What happened instead? (Actual behavior)
Git grep -f reads the filename, relative to the Git root and searches
for lines matching `tig` in the xdiff directory.
What's different between what you expected and what actually happened?
The file that Git grep uses for patterns is read relative to the root
of the Git repository, and not the current directory.
Anything else you want to add:
Please review the rest of the bug report below.
You can delete any lines you don't wish to share.
[System Info]
git version:
git version 2.42.0
cpu: x86_64
no commit associated with this build
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
uname: Linux 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27
02:56:13 UTC 2023 x86_64
compiler info: gnuc: 11.4
libc info: glibc: 2.35
$SHELL (typically, interactive shell): /bin/bash
^ permalink raw reply
* Re: [PATCH] send-email: add --compose-cover option
From: Ben Dooks @ 2023-10-12 13:07 UTC (permalink / raw)
To: git, gitster
In-Reply-To: <20231012112743.2756259-1-ben.dooks@codethink.co.uk>
On 12/10/2023 12:27, Ben Dooks wrote:
> Adding an option to automatically compose a cover letter would be
> helpful to put the whole process of sending an series with a cover
> into one command.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> Documentation/git-send-email.txt | 5 +++++
> git-send-email.perl | 11 ++++++++++-
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 41cd8cb424..f299732867 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -78,6 +78,11 @@ Missing From or In-Reply-To headers will be prompted for.
> +
> See the CONFIGURATION section for `sendemail.multiEdit`.
>
> +--compose-cover::
> + Invoke a text editor (see GIT_EDITOR in linkgit:git-var[1])
> + to edit a cover letter generated by passing --cover-letter to
> + git-send-email before invoking the editor.
> +
OOPS, clearly meant git-format-patch here, not git-send-email.
> --from=<address>::
> Specify the sender of the emails. If not specified on the command line,
> the value of the `sendemail.from` configuration option is used. If
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5861e99a6e..debec088f6 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -54,6 +54,7 @@ sub usage {
> --in-reply-to <str> * Email "In-Reply-To:"
> --[no-]xmailer * Add "X-Mailer:" header (default).
> --[no-]annotate * Review each patch that will be sent in an editor.
> + --compose-cover * Open an editor on format-patch --cover-letter
> --compose * Open an editor for introduction.
> --compose-encoding <str> * Encoding to assume for introduction.
> --8bit-encoding <str> * Encoding to assume 8bit mails if undeclared
> @@ -199,7 +200,7 @@ sub format_2822_time {
> # Variables we fill in automatically, or via prompting:
> my (@to,@cc,@xh,$envelope_sender,
> $initial_in_reply_to,$reply_to,$initial_subject,@files,
> - $author,$sender,$smtp_authpass,$annotate,$compose,$time);
> + $author,$sender,$smtp_authpass,$annotate,$compose_cover,$compose,$time);
> # Things we either get from config, *or* are overridden on the
> # command-line.
> my ($no_cc, $no_to, $no_bcc, $no_identity);
> @@ -512,6 +513,7 @@ sub config_regexp {
> "no-smtp-auth" => sub {$smtp_auth = 'none'},
> "annotate!" => \$annotate,
> "no-annotate" => sub {$annotate = 0},
> + "compose-cover" => \$compose_cover,
> "compose" => \$compose,
> "quiet" => \$quiet,
> "cc-cmd=s" => \$cc_cmd,
> @@ -782,6 +784,9 @@ sub is_format_patch_arg {
> die __("Cannot run git format-patch from outside a repository\n")
> unless $repo;
> require File::Temp;
> + if ($compose_cover) {
> + push @rev_list_opts, "--cover-letter";
> + }
> push @files, $repo->command('format-patch', '-o', File::Temp::tempdir(CLEANUP => 1), @rev_list_opts);
> }
>
> @@ -854,6 +859,8 @@ sub get_patch_subject {
>
> if ($annotate) {
> do_edit($compose_filename, @files);
> + } elsif ($compose_cover) {
> + do_edit($files[0]);
> } else {
> do_edit($compose_filename);
> }
> @@ -927,6 +934,8 @@ sub get_patch_subject {
>
> } elsif ($annotate) {
> do_edit(@files);
> +} elsif ($compose_cover) {
> + do_edit($files[0]);
> }
>
> sub term {
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ 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