* Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
From: Kevin Daudt @ 2016-09-21 17:45 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq7fa59mti.fsf@gitster.mtv.corp.google.com>
On Wed, Sep 21, 2016 at 10:36:57AM -0700, Junio C Hamano wrote:
> Kevin Daudt <me@ikke.info> writes:
>
> > On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote:
> >>
> >> * kd/mailinfo-quoted-string (2016-09-19) 2 commits
> >> - mailinfo: unescape quoted-pair in header fields
> >> - t5100-mailinfo: replace common path prefix with variable
> >
> > Is this good enough, or do you want me to look into the feedback from
> > jeff?
>
> If you are talking about the simplified loop that deliberately sets
> a rule that is looser than RFC, yes, I'd like to see you at least
> consider the pros and cons of his approach, which looked nicer to my
> brief reading of it.
>
> It is perfectly OK by me (it may not be so if you ask Peff) if you
> decide that your version is better after doing so, though.
>
> Thanks.
Alright, I'll look into it.
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
From: Junio C Hamano @ 2016-09-21 17:36 UTC (permalink / raw)
To: Kevin Daudt; +Cc: git
In-Reply-To: <20160921162628.GA27363@ikke.info>
Kevin Daudt <me@ikke.info> writes:
> On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote:
>>
>> * kd/mailinfo-quoted-string (2016-09-19) 2 commits
>> - mailinfo: unescape quoted-pair in header fields
>> - t5100-mailinfo: replace common path prefix with variable
>
> Is this good enough, or do you want me to look into the feedback from
> jeff?
If you are talking about the simplified loop that deliberately sets
a rule that is looser than RFC, yes, I'd like to see you at least
consider the pros and cons of his approach, which looked nicer to my
brief reading of it.
It is perfectly OK by me (it may not be so if you ask Peff) if you
decide that your version is better after doing so, though.
Thanks.
^ permalink raw reply
* Re: [PATCH v3 0/3] handle multiline in-body headers
From: Junio C Hamano @ 2016-09-21 17:24 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git
In-Reply-To: <cover.1474391594.git.jonathantanmy@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
> With the above change, it is actually no longer necessary to make
> is_scissors_line take plain char * (the second patch) - I think that
> that patch still improves the code, but let me know if you want me to
> remove it from this patch set.
I agree with you that it is an independently good change. Let's
keep it.
Overall looked very good. Thanks, will queue.
^ permalink raw reply
* Re: [PATCH 1/6] i18n: commit: mark message for translation
From: Junio C Hamano @ 2016-09-21 17:17 UTC (permalink / raw)
To: Jean-Noël AVILA
Cc: Vasco Almeida, git, Jiang Xin,
Ævar Arnfjörð Bjarmason
In-Reply-To: <1579845.fzuqLqrEJ8@cayenne>
Jean-Noël AVILA <jn.avila@free.fr> writes:
> Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt>
> Signed-off-by: Jean-Noel Avila <jn.avila@free.fr>
> ---
>
> Instead of distillating change requests, I'd better do it by
> myself. Here is the reworked version of the patch.
>
It would have helped if you had an in-body header to retitle this
patch for 3/6, instead of leaving it as 1/6 for "commit" X-<.
Will tweak and fit it in. Thanks.
> diff.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index c6da383..494f723 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -55,6 +55,11 @@ static char diff_colors[][COLOR_MAXLEN] = {
> GIT_COLOR_NORMAL, /* FUNCINFO */
> };
>
> +static NORETURN void die_want_option(const char *option_name)
> +{
> + die(_("option '%s' requires a value"), option_name);
> +}
> +
> static int parse_diff_color_slot(const char *var)
> {
> if (!strcasecmp(var, "context") || !strcasecmp(var, "plain"))
> @@ -3325,7 +3330,7 @@ void diff_setup_done(struct diff_options *options)
> if (options->output_format & DIFF_FORMAT_NO_OUTPUT)
> count++;
> if (count > 1)
> - die("--name-only, --name-status, --check and -s are mutually exclusive");
> + die(_("--name-only, --name-status, --check and -s are mutually exclusive"));
>
> /*
> * Most of the time we can say "there are changes"
> @@ -3521,7 +3526,7 @@ static int stat_opt(struct diff_options *options, const char **av)
> if (*arg == '=')
> width = strtoul(arg + 1, &end, 10);
> else if (!*arg && !av[1])
> - die("Option '--stat-width' requires a value");
> + die_want_option("--stat-width");
> else if (!*arg) {
> width = strtoul(av[1], &end, 10);
> argcount = 2;
> @@ -3530,7 +3535,7 @@ static int stat_opt(struct diff_options *options, const char **av)
> if (*arg == '=')
> name_width = strtoul(arg + 1, &end, 10);
> else if (!*arg && !av[1])
> - die("Option '--stat-name-width' requires a value");
> + die_want_option("--stat-name-width");
> else if (!*arg) {
> name_width = strtoul(av[1], &end, 10);
> argcount = 2;
> @@ -3539,7 +3544,7 @@ static int stat_opt(struct diff_options *options, const char **av)
> if (*arg == '=')
> graph_width = strtoul(arg + 1, &end, 10);
> else if (!*arg && !av[1])
> - die("Option '--stat-graph-width' requires a value");
> + die_want_option("--stat-graph-width");
> else if (!*arg) {
> graph_width = strtoul(av[1], &end, 10);
> argcount = 2;
> @@ -3548,7 +3553,7 @@ static int stat_opt(struct diff_options *options, const char **av)
> if (*arg == '=')
> count = strtoul(arg + 1, &end, 10);
> else if (!*arg && !av[1])
> - die("Option '--stat-count' requires a value");
> + die_want_option("--stat-count");
> else if (!*arg) {
> count = strtoul(av[1], &end, 10);
> argcount = 2;
^ permalink raw reply
* Re: [PATCH 1/3] gitweb: Fix an ancient typo in v1.7.7-rc1-1-g0866786
From: Ævar Arnfjörð Bjarmason @ 2016-09-21 17:17 UTC (permalink / raw)
To: Jakub Narębski; +Cc: Git, Junio C Hamano
In-Reply-To: <8b43ba22-9634-e2b8-8de7-d70c258f4034@gmail.com>
On Wed, Sep 21, 2016 at 7:14 PM, Jakub Narębski <jnareb@gmail.com> wrote:
> W dniu 21.09.2016 o 16:17, Ævar Arnfjörð Bjarmason napisał:
>> On Wed, Sep 21, 2016 at 3:33 PM, Jakub Narębski <jnareb@gmail.com> wrote:
>>> W dniu 21.09.2016 o 13:44, Ævar Arnfjörð Bjarmason napisał:
> [...]
>
>>>> -# Sanitize for use in XHTML + application/xml+xhtm (valid XML 1.0)
>>>> +# Sanitize for use in XHTML + application/xml+xhtml (valid XML 1.0)
>>>
>>> Nb. I wonder how common is use of XHTML nowadays, with HTML5 as standard...
>>
>> It's sent to modern browsers, I noticed it because when doing the rest
>> of the patches in the series the slightest mistake in the HTML syntax
>> would cause the page not to render in Chrome, because
>> application/xml+xhtml activates its anal parsing mode.
>
> What I wanted to say is if we should support XHTML mimetype at all;
> the future is HTML5 and perhaps gitweb should always use 'text/html'.
Regardless of what MIME type we'd normally use, as long as browsers
support application/xml+xhtml developing with it is very handy,
because you get the instant equivalent of compile errors for your
HTML, as opposed to the usual behavior of "oh this doesn't parse, but
let's try to make sense of it anyway", which often leads to fruitless
debugging sessions just because you forgot to close some tag or
quotation.
^ permalink raw reply
* Re: [PATCH 1/3] gitweb: Fix an ancient typo in v1.7.7-rc1-1-g0866786
From: Jakub Narębski @ 2016-09-21 17:14 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Git, Junio C Hamano
In-Reply-To: <CACBZZX4QDTNYDpOF+GsK3tMQamoXQhJoXuPTgB3O+iEW3jBxag@mail.gmail.com>
W dniu 21.09.2016 o 16:17, Ævar Arnfjörð Bjarmason napisał:
> On Wed, Sep 21, 2016 at 3:33 PM, Jakub Narębski <jnareb@gmail.com> wrote:
>> W dniu 21.09.2016 o 13:44, Ævar Arnfjörð Bjarmason napisał:
[...]
>>> -# Sanitize for use in XHTML + application/xml+xhtm (valid XML 1.0)
>>> +# Sanitize for use in XHTML + application/xml+xhtml (valid XML 1.0)
>>
>> Nb. I wonder how common is use of XHTML nowadays, with HTML5 as standard...
>
> It's sent to modern browsers, I noticed it because when doing the rest
> of the patches in the series the slightest mistake in the HTML syntax
> would cause the page not to render in Chrome, because
> application/xml+xhtml activates its anal parsing mode.
What I wanted to say is if we should support XHTML mimetype at all;
the future is HTML5 and perhaps gitweb should always use 'text/html'.
But this is unrelated to this change...
--
Jakub Narębski
^ permalink raw reply
* Re: [PATCH v2] ls-files: add pathspec matching for submodules
From: Junio C Hamano @ 2016-09-21 17:12 UTC (permalink / raw)
To: Brandon Williams; +Cc: git, Nguyễn Thái Ngọc Duy
In-Reply-To: <CAKoko1qS0+DgnMeNnjayEK3sWnvpuiS4oRDBSR=6s8i4okQ_Hw@mail.gmail.com>
Brandon Williams <bmwill@google.com> writes:
> On a similar but slightly different note. In general do we want
> the pathspec '??b' to match against the sib/ directory and
> subsequently have ls-files print all entries inside of the sib/
> directory? (this is in the non-recursive case)
I'd need to find time to dig a bit of history before I can give a
firm opinion on this, but here is a knee-jerk version of my reaction.
* A pathspec element that matches literally to a directory causes
itself and everything underneath the directory match that
element, e.g. "sib" would be considered a match.
* Otherwise, a pathspec that matches with the whole path as a
pattern matches the path, e.g. "??b" would match "sib" itself,
but not "sib/file". Note that "??b*" would match "sib" and
"sib/file" because the pattern match is without FNM_PATNAME
unless ':(glob)' magic is in effect.
Historically, some commands treated a pathspec as purely a prefix
match (i.e. the former) and did not use _any_ pattern matching,
while other commands did both of the above two (e.g. compare ls-tree
and ls-files). I thought we were slowly moving towards unifying
them, but apparently 'git log -- "D?cumentation"' does not show
anything close to what 'git log -- Documentation' gives us even in
today's Git.
Probably we want to change it at some point so that a pattern that
matches one leading directory would cause everything underneath to
match, e.g. "??b" would include "sib/file" just because "sib" would.
^ permalink raw reply
* Re: [PATCH 3/3] gitweb: Link to "git describe"'d commits in log messages
From: Jakub Narębski @ 2016-09-21 17:09 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git; +Cc: Junio C Hamano
In-Reply-To: <20160921114428.28664-3-avarab@gmail.com>
W dniu 21.09.2016 o 13:44, Ævar Arnfjörð Bjarmason napisał:
> Change the log formatting function to know about "git describe" output
> like v2.8.0-4-g867ad08 in addition to just plain 867ad08.
All right, that is a good plan.
>
> This also fixes a micro-regression in my change of the minimum SHA1
> length from 8 to 7, which is that dated tags like
> hadoop-20160921-113441-20-g094fb7d would start thinking the "20160921"
> part was a commit.
Actually 20160921 is 8 characters, so assuming that '-' is treated
as word boundary by Perl, it is not a regression; this false positive
was there. The new feature would help, instead of linking false match
it links whole git-describe output.
So this paragraph needs to be changed wrt. the above.
Note that there are quite a bit of shortened SHA-1 that are composed
entirely from digits, without a-f characters.
>
> There are still many valid refnames that we don't link to
> e.g. v2.10.0-rc1~2^2~1 is also a valid way to refer to
> v2.8.0-4-g867ad08, but I'm not supporting that with this commit,
> similarly it's trivially possible to create some refnames like
> "æ/var-gf6727b0" or whatever which won't be picked up by this regex.
Hopefully hierarchical tags are rare. We need to reduce false
positives.
>
> There's surely room for improvement here, but I just wanted to address
> the very common case of sticking "git describe" output into commit
> messages without trying to link to all possible refnames, that's going
> to be a rather futile exercise given that this is free text, and it
> would be prohibitively expensive to look up whether the references in
> question exist in our repository.
Note that we do not ask Git at the time of displaying commit message
if the link is valid for performance reasons; we link it, and the link
may be invalid if it was a false positive.
Note that recommended way to refer to other commit in commit mesages
is (see Documentation/SubmittingPatches):
If you want to reference a previous commit in the history of a stable
branch, use the format "abbreviated sha1 (subject, date)",
with the subject enclosed in a pair of double-quotes, like this:
Commit f86a374 ("pack-bitmap.c: fix a memleak", 2015-03-30)
noticed that ...
Hmmm... this makes previous commit even more important.
> ---
> gitweb/gitweb.perl | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 101dbc0..3a52bc7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2036,10 +2036,24 @@ sub format_log_line_html {
> my $line = shift;
>
> $line = esc_html($line, -nbsp=>1);
> - $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
> + $line =~ s{
> + \b
> + (
> + # The output of "git describe", e.g. v2.10.0-297-gf6727b0
> + # or hadoop-20160921-113441-20-g094fb7d
All right, for more complex regular expressions using in-line comments
(extended regexp in Perl) is a good idea.
> + (?<!-) # see strbuf_check_tag_ref(). Tags can't start with -
> + [A-Za-z0-9.-]+
> + (?!\.) # refs can't end with ".", see check_refname_format()
If we can assume that tag name is at least two characters (instead of
at least one character), we could get rid of those extended regexp
lookaround assertions:
(?<!pattern) - zero-width negative lookbehind assertion
(?!pattern) - zero-width negative lookahead assertion
That is:
+ [A-Za-z0-9.] # see strbuf_check_tag_ref(). Tags can't start with -
+ [A-Za-z0-9.-]*
+ [A-Za-z0-9-] # refs can't end with ".", see check_refname_format()
Also, the canonical documentation for what is allowed in refnames
is git-check-ref-format(1)... though it does not look like it includes
"tags cannot start with '-'".
Anyway, perhaps 'is it valid refname' could be passed to a subroutine,
or a named regexp (which might be more involved, like disallowing two
consecutive dots, e.g. "(?!.*\.{2})" at beginning).
> + -g[0-9a-fA-F]{7,40}
If we are limiting to git-describe output, we can get rid of A-F here.
> + |
> + # Just a normal looking Git SHA1
> + [0-9a-fA-F]{7,40}
> + )
> + \b
> + }{
> $cgi->a({-href => href(action=>"object", hash=>$1),
> -class => "text"}, $1);
> - }eg;
> + }egx;
>
> return $line;
> }
>
Good work.
I assume that you are using git-describe output in commit messages
a lot, isn't it?
--
Jakub Narębski
^ permalink raw reply
* Re: v2.10.0: ls-tree exit status is always 0, this differs from ls(1)
From: Junio C Hamano @ 2016-09-21 16:52 UTC (permalink / raw)
To: Steffen Nurpmeso; +Cc: git
In-Reply-To: <20160921132655.h49HMsHbp%steffen@sdaoden.eu>
Steffen Nurpmeso <steffen@sdaoden.eu> writes:
> I think this behaviour contradicts the manual which strongly links
> ls-tree to ls(1):
Patches to the documentation is very much welcomed.
Somewhere the similarity must end, and actually it ends a lot
earlier, as "/bin/ls" takes exact paths while "ls-tree" (or any
other Git command for that matter) takes a pathspec pattern,
and not having a path that matches the pathspec pattern is not
an error condition.
Thanks.
^ permalink raw reply
* Re: [PATCH 3/3] gitweb: Link to "git describe"'d commits in log messages
From: Junio C Hamano @ 2016-09-21 16:50 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: git, Jakub Narebski, Kay Sievers
In-Reply-To: <20160921114428.28664-3-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> There's surely room for improvement here, but I just wanted to address
> the very common case of sticking "git describe" output into commit
> messages without trying to link to all possible refnames, that's going
> to be a rather futile exercise given that this is free text, and it
> would be prohibitively expensive to look up whether the references in
> question exist in our repository.
When I saw 2/3 I wondered about one thing and 3/3 shares the same,
which is that we only use regex match and do not validate for a
false match. Would it be too expensive to pick up what _looks_ like
a rev (e.g. hex or g(refname regexp)-hex) then validate it with
"rev-parse --verify --quiet" to make sure it is a rev, before
actually making it a link? Even if are we trying to account for
people referring to commits that do not exist in this repository
(e.g. some other project, in a submodule repository, or just an
earlier incarnation of rebasing that has since been lost), it seems
to me that it does not help to mark them with a link that won't
resolve.
> ---
> gitweb/gitweb.perl | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 101dbc0..3a52bc7 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2036,10 +2036,24 @@ sub format_log_line_html {
> my $line = shift;
>
> $line = esc_html($line, -nbsp=>1);
> - $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
> + $line =~ s{
> + \b
> + (
> + # The output of "git describe", e.g. v2.10.0-297-gf6727b0
> + # or hadoop-20160921-113441-20-g094fb7d
> + (?<!-) # see strbuf_check_tag_ref(). Tags can't start with -
> + [A-Za-z0-9.-]+
> + (?!\.) # refs can't end with ".", see check_refname_format()
> + -g[0-9a-fA-F]{7,40}
> + |
> + # Just a normal looking Git SHA1
> + [0-9a-fA-F]{7,40}
> + )
> + \b
> + }{
> $cgi->a({-href => href(action=>"object", hash=>$1),
> -class => "text"}, $1);
> - }eg;
> + }egx;
>
> return $line;
> }
^ permalink raw reply
* Re: [PATCH v1] travis-ci: ask homebrew for the its path instead of hardcoding it
From: Junio C Hamano @ 2016-09-21 16:42 UTC (permalink / raw)
To: Lars Schneider; +Cc: stefan.naewe, git
In-Reply-To: <14C21417-491B-4C8E-90FE-47154EEDD9FE@gmail.com>
Lars Schneider <larsxschneider@gmail.com> writes:
>> On 21 Sep 2016, at 11:31, stefan.naewe@atlas-elektronik.com wrote:
>>
>> In the Subject: s/the //
>>
>> Am 21.09.2016 um 10:45 schrieb larsxschneider@gmail.com:
>>> From: Lars Schneider <larsxschneider@gmail.com>
>>>
>>> The TravisCI macOS build is broken because homebrew (a macOS depedency
>>
>> s/depedency/dependency/
>
> Thanks for spotting both errors!
>
> @Junio: Should I make a v2?
No. osx before_install stuff was in there since the very beginning,
i.e. 522354d7 ("Add Travis CI support", 2015-11-27), so I guess this
needs to go to maint-2.7 and upwards, but I guess we should discourage
people to stay on an older maintenance track forever, so let's do
this only for 'maint' and upwards.
^ permalink raw reply
* Re: [PATCH] gitweb: use highlight's shebang detection
From: Junio C Hamano @ 2016-09-21 16:38 UTC (permalink / raw)
To: Jakub Narębski; +Cc: Ian Kelling, git
In-Reply-To: <108ce713-337a-801a-6c3b-089ef25a3883@gmail.com>
Jakub Narębski <jnareb@gmail.com> writes:
> W dniu 06.09.2016 o 21:00, Ian Kelling pisze:
>
>> The highlight binary can detect language by shebang when we can't tell
>> the syntax type by the name of the file.
>
> Was it something always present among highlight[1] binary capabilities,
> or is it something present only in new enough highlight app? Or only
> in some specific fork / specific binary? I couldn't find language
> detection in highlight[1] documentation...
> ...
> Thank you for your work on this patch,
Thanks for reviewing. It seems that there will be further exchange
needed before I can pick it up?
^ permalink raw reply
* Re: v2.9.3 and v2.10.0: `name-ref' HEAD gives wrong branch name
From: Junio C Hamano @ 2016-09-21 16:37 UTC (permalink / raw)
To: Jakub Narębski; +Cc: Bryan Turner, Steffen Nurpmeso, Git Users
In-Reply-To: <b1d35732-4b4c-e0c1-59da-44d3e3b28799@gmail.com>
Jakub Narębski <jnareb@gmail.com> writes:
>> Have you tried "git symbolic-ref HEAD"?
>>
>> $ git symbolic-ref HEAD
>> refs/heads/master
>>
>> If you don't want the fully-qualified ref, you can add --short:
>>
>> $ git symbolic-ref --short HEAD
>> master
>
> This does not work for detached HEAD, but perhaps you don't need
> to worry about this.
I am not sure what you mean by "does not work". Asking what ref
HEAD points at to symbolic-ref will tell you it does not point at
anything by exiting with non-zero status and that can be relied
upon.
Asking "symbolic-ref HEAD" has been the way how "git branch" and
other commands find out what branch is currently checked out for
almost eternity ("git symbolic-ref" appeared in Git v0.99.8).
^ permalink raw reply
* Re: [PATCH 2/3] gitweb: Link to 7-character SHA1SUMS in commit messages
From: Jakub Narębski @ 2016-09-21 16:26 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git; +Cc: Junio C Hamano
In-Reply-To: <20160921114428.28664-2-avarab@gmail.com>
W dniu 21.09.2016 o 13:44, Ævar Arnfjörð Bjarmason napisał:
> Subject: [PATCH 2/3] gitweb: Link to 7-character SHA1SUMS in commit messages
This is modification of a feature, not a new feature it sounds like.
I think the following title / subject would be better:
Subject: [PATCH 2/3] gitweb: Link to 7-char+ SHA1s, not only 8-char+
>
> Change the minimum length of a commit we'll link to from 8 to 7.
I think it would read better as:
Change the minimum length of an abbreviated object identifier in the
commit message gitweb tries to turn into link from 8 hexchars to 7.
>
> This arbitrary minimum length of 8 was introduced in
> v1.4.4.2-151-gbfe2191, but as seen in e.g. v1.7.4-1-gdce9648 the
> default abbreviation length is 7.
Right. I wonder why it was 8 in gitweb...
>
> It's still possible to reference SHA1s down to 4 characters in length,
> see v1.7.4-1-gdce9648's MINIMUM_ABBREV, but I can't see how to make
> git actually produce that, so I doubt anyone is putting that into log
> messages in practice, but people definitely do put 7 character SHA1s
> into log messages.
There is an additional problem: the shorter SHA1 abbrev we try to
match, the more possibility of false positives, words that only look
like (shortened SHA-1).
For 7 characters there is at last one word that can be mistaken
for SHA1 abbrev, namely 'deedeed' (hopefully rare in commit messages).
For 6 characters we have 'accede', 'beaded', 'decade' (!), 'deface',
'facade' (!!), and possibly more (and of course all 7 character
hexdigit words).
Also, the number of digits provided as an optional parameter to
--abbrev or --abbrev-commit options is only a minimal number of
hexdigits: Git would use as many as needed for the abbreviated SHA-1
to be unambiguous, at current time.
I think allowing 7-character shortened SHA-1, which is what Git
produces for smaller repositories by default is (might be?) a good
idea. Thanks for the patch.
>
> I think it's fairly dubious to link to things matching [0-9a-fA-F]
> here as opposed to just [0-9a-f], that dates back to the initial
> version of gitweb from 161332a. Git will accept all-caps SHA1s, but
> didn't ever produce them as far as I can tell.
All right, thanks for reminder.
Signoff?
> ---
> gitweb/gitweb.perl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 9473daf..101dbc0 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -2036,7 +2036,7 @@ sub format_log_line_html {
> my $line = shift;
>
> $line = esc_html($line, -nbsp=>1);
> - $line =~ s{\b([0-9a-fA-F]{8,40})\b}{
> + $line =~ s{\b([0-9a-fA-F]{7,40})\b}{
> $cgi->a({-href => href(action=>"object", hash=>$1),
> -class => "text"}, $1);
> }eg;
>
Nice and simple.
P.S. I have reworking of commit message parsing and enhancement in my
long, long and dated gitweb TODO list :-(
P.P.S. Kay Sievers no longer works on gitweb, and I think no longer
works at SuSE but at RedHat.
Best,
--
Jakub Narębski
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
From: Kevin Daudt @ 2016-09-21 16:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqd1jzcvs5.fsf@gitster.mtv.corp.google.com>
On Mon, Sep 19, 2016 at 04:30:34PM -0700, Junio C Hamano wrote:
>
> * kd/mailinfo-quoted-string (2016-09-19) 2 commits
> - mailinfo: unescape quoted-pair in header fields
> - t5100-mailinfo: replace common path prefix with variable
Is this good enough, or do you want me to look into the feedback from
jeff?
^ permalink raw reply
* Re: 2.10.0: git log --oneline prints gpg signatures in 4 lines
From: Junio C Hamano @ 2016-09-21 16:26 UTC (permalink / raw)
To: Jeff King; +Cc: Leandro Lucarella, Mehul Jain, git
In-Reply-To: <20160920231533.fxcdjgphz25hnbxt@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I don't think anything has changed here in 2.10. Running "git log
> --oneline --show-signature" has _always_ been horribly ugly. However,
> 2.10 did introduce the "log.showsignature" config, which makes "git log
> --oneline" pretty unusable when it is enabled. Ditto for one-liner uses
> of "--format".
>
> I think we should probably ignore the config entirely when using any of
> the one-liner formats (and I'd include --format, too, even though it can
> sometimes be multi-line; it already has %GG to include that information
> as appropriate).
>
> Another option would be to somehow represent the signature information
> in the --oneline output, but I think I'd rather leave that for people to
> experiment with using "--format".
My thinking is that "--oneline --show-signature" and "--oneline"
with log.showsignature set to true without "--no-show-signature" on
the command line should produce identical result.
The current definition of "--oneline" seems to me "the commit object
name and the log message is shown on a single line" (I consider that
the decoration given by "--decorate" is part of "commit object
name"), but there may be other things shown that may not fit on a
single line.
I do not terribly mind changing the definition of "--oneline" to
"what is output is ONLY the commit object name and the log message,
nothing else is shown", though. After all, the output from "log" is
for human consumption, and it is a bug in the script if it is
depending on parsing the "log" output, so it is OK to change its
output to suit human needs, if necessary.
^ permalink raw reply
* Re: 2.10.0: git log --oneline prints gpg signatures in 4 lines
From: Junio C Hamano @ 2016-09-21 16:19 UTC (permalink / raw)
To: Leandro Lucarella; +Cc: git
In-Reply-To: <20160920170954.38b24284@labs-064.localdomain>
Leandro Lucarella <leandro.lucarella@sociomantic.com> writes:
> Hi, starting from 2.10.0 I noticed that when using git log --oneline,
> if commits are signed with GPG, now the signatures are printed too, and
> it takes 3 lines for the signature information + 1 line for the title
> of the commit, so suddenly --oneline became --fourline :)
>
> Is this really intended?
I think so. The documentation for --oneline may say "one line per
commit" but in reality, some things cannot be expressed on one line.
It should probably be described as "commit object name and its log
message is formatted to be on a single line" without limiting other
things that the user may have asked to be shown.
And show-signature is an example of what the user additionally can
ask that cannot be fit on a single line. There probably are others.
^ permalink raw reply
* Re: clarification of `rev-list --no-walk ^<rev>`?
From: Junio C Hamano @ 2016-09-21 16:15 UTC (permalink / raw)
To: Michael J Gruber; +Cc: Philip Oakley, Git List
In-Reply-To: <affca640-4ed8-268a-9f7d-e2c7ce6d66fc@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> I think you answered to e-mail (in-reply-to) and to Philip's actual text
> (quotes), but just in case:
Yes, my mistake. Sorry for the noise.
^ permalink raw reply
* Re: Bug: pager.<cmd> doesn't work well with editors
From: Junio C Hamano @ 2016-09-21 16:15 UTC (permalink / raw)
To: Jeff King; +Cc: Anatoly Borodin, git
In-Reply-To: <20160920014733.7whjuxfuimx5ztdb@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> And this isn't really limited to the editor. It's more _annoying_ with
> the editor, but really "pager.tag" does not make any sense to set right
> now, because it is handled outside of the "tag" command entirely, and
> doesn't know what mode the tag command will be running in.
Stepping back even further, perhaps the whole pager.<cmd> was a bad
interim move. For those who set "less" without "-F", being able to
set pager.<cmd> to false may still be necessary, but I am wondering
about setting it to true or a command string here.
It did mean well and may have helped when "git <cmd>" that produces
reams of output had not yet learned to auto-paginate as a stop-gap
measure by allowing users to set pager.<cmd>, but I wonder if the
ideal course of action was to identify (or "wait until people show
their desire") individual operating modes of various commands and
teach them to auto-paginate. For example, "tag -l" may be one of
them that we would want to teach to.
^ permalink raw reply
* Re: [PATCH] mailinfo: unescape quoted-pair in header fields
From: Junio C Hamano @ 2016-09-21 16:07 UTC (permalink / raw)
To: Jeff King; +Cc: Kevin Daudt, git, Swift Geek
In-Reply-To: <20160920035710.qw2byl3qeqwih7t5@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> So if that's the case, do we actually need to care if we see any
> parenthesized comments? I think we should just leave comments in place
> either way, so syntactically they are only interesting insofar as we
> replace quoted pairs or not.
>
> IOW, I wonder if:
>
> while ((c = *in++)) {
> switch (c) {
> case '\\':
> if (!*in)
> return 0; /* ignore trailing backslash */
> /* quoted pair */
> strbuf_addch(out, *in++);
> break;
> case '"':
> /*
> * This may be starting or ending a quoted section,
> * but we do not care whether we are in such a section.
> * We _do_ need to remove the quotes, though, as they
> * are syntactic.
> */
> break;
> default:
> /*
> * Anything else is a normal character we keep. These
> * _might_ be violating the RFC if they are magic
> * characters outside of a quoted section, but we'd
> * rather be liberal and pass them through.
> */
> strbuf_addch(out, c);
> break;
> }
> }
>
> would work. I certainly do not mind following the RFC more closely, but
> AFAICT the very simple code above gives a pretty forgiving outcome.
The simplicity of the code does look attractive to me. I do not
offhand see an obvious case/flaw that this simplified rule would
mangle a valid human-readable part.
^ permalink raw reply
* Re: [PATCH v3] format-patch: Add --rfc for the common case of [RFC PATCH]
From: Junio C Hamano @ 2016-09-21 15:57 UTC (permalink / raw)
To: Josh Triplett; +Cc: git, Andrew Donnellan, Jeff King
In-Reply-To: <501a6bfb2a70f44f080b2f119e4503ccbf88f639.1474330487.git-series.josh@joshtriplett.org>
Josh Triplett <josh@joshtriplett.org> writes:
> This provides a shorter and more convenient alias for
> --subject-prefix='RFC PATCH'.
Shorter and more convenient is quite subjective but more important
as a justification is that we believe [RFC PATCH] is used fairly
commonly (at least in certain circles).
> Includes documentation in the format-patch manpage, and a new test
> covering --rfc.
We can see that from diffstat ;-)
I'd retitle this like so:
format-patch: add "--rfc" for the common case of [RFC PATCH]
Add an alias for --subject-prefix='RFC PATCH', which is used
commonly in some development communities to deserve such a
short-hand.
Signed-off-by: Josh Triplett <josh@joshtriplett.org>
Reviewed-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 9624c84..9b200b3 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -19,7 +19,8 @@ SYNOPSIS
> [--start-number <n>] [--numbered-files]
> [--in-reply-to=Message-Id] [--suffix=.<sfx>]
> [--ignore-if-in-upstream]
> - [--subject-prefix=Subject-Prefix] [(--reroll-count|-v) <n>]
> + [--rfc] [--subject-prefix=Subject-Prefix]
> + [(--reroll-count|-v) <n>]
> [--to=<email>] [--cc=<email>]
> [--[no-]cover-letter] [--quiet] [--notes[=<ref>]]
> [<common diff options>]
> @@ -172,6 +173,11 @@ will want to ensure that threading is disabled for `git send-email`.
> allows for useful naming of a patch series, and can be
> combined with the `--numbered` option.
>
> +--rfc::
> + Alias for `--subject-prefix="RFC PATCH"`. RFC means "Request For
> + Comments"; use this when sending an experimental patch for
> + discussion rather than application.
> +
I do not think we want to be in the business of encouragign or
discouraging the use of "[RFC PATCH]".
--rfc:: A short-hand for `--subject-prefix="RFC PATCH"`.
RFC stands for "request for comments" and such a
prefix is used in some development communities when
sending a patch primarily to illustrate an idea to
help discussion, rather than to be applied.
perhaps?
The code and test both look good to me.
Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
From: Junio C Hamano @ 2016-09-21 15:44 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Git Mailing List
In-Reply-To: <CACsJy8Dwe1n-YQfNMOyduLksNhAkAEPpBE3S0n==T-D1E0BneA@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Tue, Sep 20, 2016 at 6:30 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> * nd/checkout-disambiguation (2016-09-09) 4 commits
>> - fixup! checkout.txt: document a common case that ignores ambiguation rules
>> - checkout: fix ambiguity check in subdir
>> - checkout.txt: document a common case that ignores ambiguation rules
>> - checkout: add some spaces between code and comment
>>
>> "git checkout <word>" does not follow the usual disambiguation
>> rules when the <word> can be both a rev and a path, to allow
>> checking out a branch 'foo' in a project that happens to have a
>> file 'foo' in the working tree without having to disambiguate.
>> This was poorly documented and the check was incorrect when the
>> command was run from a subdirectory.
>>
>> Waiting for an Ack for fixup!
>
> Oops, I didn't know (I have about 300 unread git mails in my inbox), Ack.
Thanks.
^ permalink raw reply
* Re: What's cooking in git.git (Sep 2016, #05; Mon, 19)
From: Junio C Hamano @ 2016-09-21 15:40 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git
In-Reply-To: <alpine.DEB.2.20.1609202349260.129229@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> * jk/rebase-i-drop-ident-check (2016-07-29) 1 commit
>> (merged to 'next' on 2016-08-14 at 6891bcd)
>> + rebase-interactive: drop early check for valid ident
>>
>> Even when "git pull --rebase=preserve" (and the underlying "git
>> rebase --preserve") can complete without creating any new commit
>> (i.e. fast-forwards), it still insisted on having a usable ident
>> information (read: user.email is set correctly), which was less
>> than nice. As the underlying commands used inside "git rebase"
>> would fail with a more meaningful error message and advice text
>> when the bogus ident matters, this extra check was removed.
>>
>> Will hold to see if people scream.
>> cf. <20160729224944.GA23242@sigill.intra.peff.net>
>
> Let's do this.
We have already been doing it (i.e. "hold to see if people scream")
for some time.
Does it conflict with your effort to reimplement "rebase -i" in C to
keep this in 'next'? Do you want it to move to 'master'? I was
under the impression that it would not make a difference to have or
not have this patch once your reimplementation gets merged (meaning:
the removal of the three lines will be done by wholesale removal of
git-rebase--interactive.sh done the endgame of your series), so...
^ permalink raw reply
* Re: 2.10.0: git log --oneline prints gpg signatures in 4 lines
From: Michael J Gruber @ 2016-09-21 15:05 UTC (permalink / raw)
To: Leandro Lucarella, Jeff King; +Cc: Mehul Jain, git
In-Reply-To: <20160921155300.79252e68@labs-064.localdomain>
Leandro Lucarella venit, vidit, dixit 21.09.2016 15:53:
> On Tue, 20 Sep 2016 19:15:33 -0400
> Jeff King <peff@peff.net> wrote:
>
>> On Tue, Sep 20, 2016 at 05:09:54PM +0200, Leandro Lucarella wrote:
>>
>>> Hi, starting from 2.10.0 I noticed that when using git log
>>> --oneline, if commits are signed with GPG, now the signatures are
>>> printed too, and it takes 3 lines for the signature information + 1
>>> line for the title of the commit, so suddenly --oneline became
>>> --fourline :)
>>>
>>> Is this really intended?
>>
>> I don't think anything has changed here in 2.10. Running "git log
>> --oneline --show-signature" has _always_ been horribly ugly. However,
>> 2.10 did introduce the "log.showsignature" config, which makes "git
>> log --oneline" pretty unusable when it is enabled. Ditto for
>> one-liner uses of "--format".
>>
>> I think we should probably ignore the config entirely when using any
>> of the one-liner formats (and I'd include --format, too, even though
>> it can sometimes be multi-line; it already has %GG to include that
>> information as appropriate).
>
> Woops! Definitely it shouldn't be added when --format is used, this is
> also breaking some scripts I have using git log --format to get some
> information about commits, and GPG information is being output even
> when there is anything about GPG requested in the chosen format.
>
> I guess I will disable log.showsignatures for now... :(
>
I guess that's one of the reasons why I didn't like that config option
to begin with. There's a flood of these config "convenience" options
lately where we have to special case for scripting... Aliases must have
become old school or something.
Note that "git log --show-notes", "git log --oneline --raw", "git log
--oneline -p" and similar are not "one line" either. So
"--show-signature" behaves just like all others.
git log --format="%h %G? %s"
or a colored version thereof may be what you want to alias as "log1" or
such.
Cheers,
Michael
^ permalink raw reply
* Re: clarification of `rev-list --no-walk ^<rev>`?
From: Michael J Gruber @ 2016-09-21 14:51 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Philip Oakley, Git List
In-Reply-To: <affca640-4ed8-268a-9f7d-e2c7ce6d66fc@drmicha.warpmail.net>
[So many typos, sorry]
Michael J Gruber venit, vidit, dixit 21.09.2016 16:46:
> Junio C Hamano venit, vidit, dixit 19.09.2016 18:12:
>> Michael J Gruber <git@drmicha.warpmail.net> writes:
>>
>>>> It can be read that
>>>>
>>>> $ git cherry-pick maint next
>>>>
>>>> would pick two single commits, while
>>>>
>>>> $ git cherry-pick maint next ^master
>>>>
>>>> could implicitly be read as
>>>>
>>>> $ git cherry-pick maint next --do-walk ^master
>>
>> You can read it as "master..next maint" that does force walking.
>>
>>>> Clearly that's not what is intended, which is
>>>>
>>>> $ git cherry-pick --do-walk maint next ^master
>>
>> I do not see the distinction betwee the above two you seem to be
>> trying to make. Care to explain?
>
> I think you answered to e-mail (in-reply-to) and to Philip's actual text
> (quotes), but just in case:
"my e-mail"
>
> [git]✓ git rev-list --no-walk ^HEAD~3 HEAD
> 47d74601f5c6bbef215a887be2ca877e34391c9f
> 574dece7b651fbae385add51d7aaea1cc414007a
> 3fbbf6e9e40b151215cce6c6e25cd4db0232d870
> [git]✓ git rev-list ^HEAD~3 --no-walk HEAD
> 47d74601f5c6bbef215a887be2ca877e34391c9f
>
> The order of revision arguments and options does play role (but where I
> put my HEAD does not, uhm), i.e. walk-options vs. negative refs.
"play a role"
"negative revs"
>
> The reason is that negative revs come with an implicit --do-walk (we
> need to walk to mark uninteresting revs), and the last
"in order to mark"
> --do-walk/--no-walk wins. That's what I meant with my comment.
>
> But there is only one walk (or none), and one setting effective for all
> revision arguments.
>
> Michael
>
^ 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