guix-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[bug#50363] [PATCH v3] Improve default diff hunk header detection.


From: Sarah Morgensen
Subject: [bug#50363] [PATCH v3] Improve default diff hunk header detection.
Date: Sun, 19 Sep 2021 15:56:43 -0700

Hello Kyle,

Thanks for taking a look and trying this out!

Kyle Meyer <kyle@kyleam.com> writes:

> Sarah Morgensen writes:
>
>> diff --git a/.gitattributes b/.gitattributes
>> new file mode 100644
>> index 0000000000..51f29b8a48
>> --- /dev/null
>> +++ b/.gitattributes
>> @@ -0,0 +1,3 @@
>> +*.scm       diff=scheme
>> +*.scm.in    diff=scheme
>> +*.texi      diff=tex
>
> "tex" is a built-in pattern for LaTeX.  To avoid confusion, I suggest
> "texinfo", which is what the Emacs repo uses.

Good point.  I was trying to use the built-in because it has a
whitespace regex defined in userdiff.c, but I just found that I can set
that with wordRegex, so I'll do that.  I'll take a look at the Emacs
repo as well.

>
>> diff --git a/doc/contributing.texi b/doc/contributing.texi
> [...]
>> +@example
>> +git config --local include.path=../etc/git/gitconfig
>> +@end example
>
> The '=' above should be a space:
>
>   $ git config --local include.path=../etc/git/gitconfig
>   error: invalid key: include.path=../etc/git/gitconfig
>
> (This is true in the other examples as well.)

Thanks for catching this.

>
> Just thinking aloud: the include.path approach above assumes a clone
> with the standard $GIT_DIR and working tree layout, but that seems fine
> because users with more unusual setups would know how to adjust it for
> their case.  Also, for git-worktree users, the etc/git/gitconfig file in
> the main worktree would be used within other worktrees, but for this use
> case I doubt that will cause confusion.

I actually wrote about the worktree limitation at first, but I thought
including it would be more confusing to people who don't use them than
it would be helpful to people who do.

>>  @unnumberedsubsec Sending a Patch Series
>>  @anchor{Sending a Patch Series}
>>  @cindex patch series
>>  @cindex @code{git send-email}
>> -@cindex @code{git-send-email}
>
> I suspect the second variant was included to help those that are using
> the git- variant to search (perhaps because that's how they'd refer to
> the manpage or because they know about Git's exec path).  Not a big deal
> either way, but keeping it around seems okay to me.

Hmmm.  I originally removed it because it results in two entries in the
index.  Perhaps I can index them as

  @cindex @code{git send-email} (or @code{git-send-email})

or similar.  What do you think?

>> diff --git a/etc/git/gitconfig b/etc/git/gitconfig
>> new file mode 100644
>> index 0000000000..290006816f
>> --- /dev/null
>> +++ b/etc/git/gitconfig
>> @@ -0,0 +1,5 @@
>> +[diff "scheme"]
>> +    xfuncname = "^(\\(define.*)$"
>> +
>> +[diff "tex"]
>> +    xfuncname = 
>> "^(@((|sub|subsub)section|(unnumbered|appendix)((|sub|subsub)sec)|(|major|chap|sub|subsub)heading|chapter|part|titlepage|top)[
>>  \t]+.*)$"
>
> Fwiw the Emacs repo sets a simpler value (via autogen.sh):
>
>   ^@node[[:space:]]+([^,[:space:]][^,]+)

I made this choice because we have several unnumbered subsections
without nodes that are fairly long.  Otherwise, nodes make a good
choice.

Also, their xfuncname will only show the node name as the diff hunk
header--not the whole line.  I also considered this (I think it's
cleaner) but I thought it might be confusing.  If others like it, let's
do it.

--
Sarah





reply via email to

[Prev in Thread] Current Thread [Next in Thread]