threads / patch / 43138

patchRe: [PATCH] gitweb: Add "next" link to commitdiff view

Subject: Re: [PATCH] gitweb: Add "next" link to commitdiff view

## tl;dr

6 messages between Oct 28, 2006 and Oct 29, 2006. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Jakub Narebski· Oct 28, 2006, 16:10 UTC · lore
Jakub Narebski wrote:
Show 20 quoted lines
> Add a kind of "next" view in the bottom part of navigation bar for
> "commitdiff" view.
> 
> For commitdiff between two commits:
>   (from: _commit_)
> For commitdiff for one single parent commit:
>   (parent: _commit_)
> For commitdiff for one merge commit
>   (merge: _commit_ _commit_ ...)
> For commitdiff for root (parentless) commit
>   (initial)
> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
> display, everything is perhaps unnecessary esc_html on display.
> 
> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> ---
> 
> Junio (and others), is it what you wanted? The idea was to change
> "commitdiff" view only in minimal way, and preserve similarity
> to "commit" format.
Any reasons not to accept this patch? I find it very useful.

The idea to use fixed string instead of shortened SHA-1 of commit was abandoned after some discussion in this thread.

-- 
Jakub Narebski
Junio C Hamano· Oct 28, 2006, 19:00 UTC · re: Jakub Narebski · lore
Jakub Narebski <jnareb@gmail.com> writes:
Show 23 quoted lines
> Jakub Narebski wrote:
>> Add a kind of "next" view in the bottom part of navigation bar for
>> "commitdiff" view.
>> 
>> For commitdiff between two commits:
>>   (from: _commit_)
>> For commitdiff for one single parent commit:
>>   (parent: _commit_)
>> For commitdiff for one merge commit
>>   (merge: _commit_ _commit_ ...)
>> For commitdiff for root (parentless) commit
>>   (initial)
>> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
>> display, everything is perhaps unnecessary esc_html on display.
>> 
>> Signed-off-by: Jakub Narebski <jnareb@gmail.com>
>> ---
>> 
>> Junio (and others), is it what you wanted? The idea was to change
>> "commitdiff" view only in minimal way, and preserve similarity
>> to "commit" format.
>
> Any reasons not to accept this patch?

Nothing other than it was lost in the noise (I am partly responsible for) that followed the patch.

	Message-ID: <200610230037.57183.jnareb@gmail.com>

Who acked this in the discussion? I'd like to add "Acked-by:" for them when I make the commit.

Luben Tuikov· Oct 29, 2006, 01:50 UTC · re: Jakub Narebski · lore
--- Jakub Narebski <jnareb@gmail.com> wrote:
Show 26 quoted lines
> Jakub Narebski wrote:
> > Add a kind of "next" view in the bottom part of navigation bar for
> > "commitdiff" view.
> > 
> > For commitdiff between two commits:
> >   (from: _commit_)
> > For commitdiff for one single parent commit:
> >   (parent: _commit_)
> > For commitdiff for one merge commit
> >   (merge: _commit_ _commit_ ...)
> > For commitdiff for root (parentless) commit
> >   (initial)
> > where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
> > display, everything is perhaps unnecessary esc_html on display.
> > 
> > Signed-off-by: Jakub Narebski <jnareb@gmail.com>
> > ---
> > 
> > Junio (and others), is it what you wanted? The idea was to change
> > "commitdiff" view only in minimal way, and preserve similarity
> > to "commit" format.
> 
> Any reasons not to accept this patch? I find it very useful.
> 
> The idea to use fixed string instead of shortened SHA-1 of commit
> was abandoned after some discussion in this thread.

I prefer using the commit-8 without any "..." postfixed. Anyone who knows squat about git knows very well what a commit-8 is when they see one -- the first 8 hexadecimal digits of the full SHA-1.

I like using "next" only when there is a "prev" right next to it,
i.e. based on _context_, something like this:
   << prev next >>
where "<< prev" is hyperlinked, and "next >>" is also.

I looked at your patch (first email in this thread) and from what I gathered that it does, I like it (haven't tried it yet).

What I like about it is the clear information it conveys: it gives me both the commit-8 and what that commit-8 _is_.

I sometimes remember commit-8s and this helps me a lot when I move the mouse pointer to an xterm and want to do git-show <commit-8>, and it futhermore helps me _know_ the relationship between that commit-8 and the rest of the information in the window frame gitweb is showing, (parent, etc).

Acked-by: Luben Tuikov <ltuikov@yahoo.com>
(not that it matters in any way ;-) )
   Luben
Jakub Narebski· Oct 29, 2006, 01:58 UTC · re: Luben Tuikov · lore
Luben Tuikov wrote:
Show 15 quoted lines
> --- Jakub Narebski <jnareb@gmail.com> wrote:
>> Jakub Narebski wrote:
>>> Add a kind of "next" view in the bottom part of navigation bar for
>>> "commitdiff" view.
>>> 
>>> For commitdiff between two commits:
>>>   (from: _commit_)
>>> For commitdiff for one single parent commit:
>>>   (parent: _commit_)
>>> For commitdiff for one merge commit
>>>   (merge: _commit_ _commit_ ...)
>>> For commitdiff for root (parentless) commit
>>>   (initial)
>>> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
>>> display, everything is perhaps unnecessary esc_html on display.
[...]
Show 14 quoted lines
>> 
>> Any reasons not to accept this patch? I find it very useful.
>> 
>> The idea to use fixed string instead of shortened SHA-1 of commit
>> was abandoned after some discussion in this thread.
> 
> I prefer using the commit-8 without any "..." postfixed.  Anyone who
> knows squat about git knows very well what a commit-8 is when they
> see one -- the first 8 hexadecimal digits of the full SHA-1.
> 
> I like using "next" only when there is a "prev" right next to it,
> i.e. based on _context_, something like this:
>    << prev next>>
> where "<< prev" is hyperlinked, and "next>>" is also.

Unfortunately this is simply not possible in this case, as links in git are only from commit to paren(a), in one direction only.

Besides as you can see we can have more than one "next>>" link: for merge commit there are multiple parents. Well, for branch points there are multiple commits which have given commit as (one of) its parent(s), so there can be multiple "<<prev" links as well.

Show 5 quoted lines
> I looked at your patch (first email in this thread) and from
> what I gathered that it does, I like it (haven't tried it yet).
> 
> What I like about it is the clear information it conveys:
> it gives me both the commit-8 and what that commit-8 _is_.
It is commit-7, but that can be easily changed.
> Acked-by: Luben Tuikov <ltuikov@yahoo.com>
> (not that it matters in any way ;-) )
IIRC Junio asked for ACKs.
-- 
Jakub Narebski
Luben Tuikov· Oct 29, 2006, 02:04 UTC · re: Jakub Narebski · lore
--- Jakub Narebski <jnareb@gmail.com> wrote:
Show 34 quoted lines
> Luben Tuikov wrote:
> > --- Jakub Narebski <jnareb@gmail.com> wrote:
> >> Jakub Narebski wrote:
> >>> Add a kind of "next" view in the bottom part of navigation bar for
> >>> "commitdiff" view.
> >>> 
> >>> For commitdiff between two commits:
> >>>   (from: _commit_)
> >>> For commitdiff for one single parent commit:
> >>>   (parent: _commit_)
> >>> For commitdiff for one merge commit
> >>>   (merge: _commit_ _commit_ ...)
> >>> For commitdiff for root (parentless) commit
> >>>   (initial)
> >>> where _link_ denotes hyperlink. SHA1 is shortened to 7 characters on
> >>> display, everything is perhaps unnecessary esc_html on display.
> [...]
> >> 
> >> Any reasons not to accept this patch? I find it very useful.
> >> 
> >> The idea to use fixed string instead of shortened SHA-1 of commit
> >> was abandoned after some discussion in this thread.
> > 
> > I prefer using the commit-8 without any "..." postfixed.  Anyone who
> > knows squat about git knows very well what a commit-8 is when they
> > see one -- the first 8 hexadecimal digits of the full SHA-1.
> > 
> > I like using "next" only when there is a "prev" right next to it,
> > i.e. based on _context_, something like this:
> >    << prev next>>
> > where "<< prev" is hyperlinked, and "next>>" is also.
> 
> Unfortunately this is simply not possible in this case, as links in git
> are only from commit to paren(a), in one direction only.

I was not suggesting that. I simply saw a "next" suggestion in this thread and wanted to mention about "next", "prev" and context.

> It is commit-7, but that can be easily changed.

Please do change it to "commit-8" -- it'd be consistent with the rest of GIT.

> > Acked-by: Luben Tuikov <ltuikov@yahoo.com>
> > (not that it matters in any way ;-) )
> 
> IIRC Junio asked for ACKs.
In this email, it seems that he's asking for, quoting,
 "Acked-by:":
http://marc.theaimsgroup.com/?l=git&m=116206206008374&w=2
      Luben
Jakub Narebski· Oct 29, 2006, 02:10 UTC · re: Luben Tuikov · lore
Luben Tuikov wrote:
Show 6 quoted lines
> --- Jakub Narebski <jnareb@gmail.com> wrote:
>>
>> It is commit-7, but that can be easily changed.
> 
> Please do change it to "commit-8" -- it'd be consistent with the rest
> of GIT.

Well, some parts of GIT use "commit-8" (for example git-describe), other use "commit-7...": this includes git-diff both in --raw (git diff --raw, as opposed to git-diff-tree), git-diff in it's patch format in "index <hash>..<hash>" extended header, in "Merge:" line for merges in git-show output.

-- 
Jakub Narebski

← back to recent threads