"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Mark Jamsek <mark@jamsek.com>
Subject:
Re: tog: keymaps to navigate to prev/next file/hunk in the diff
To:
gameoftrees@openbsd.org
Date:
Tue, 26 Jul 2022 01:51:02 +1000

Download raw body.

Thread
On 22-07-24 09:51pm, Stefan Sperling wrote:
> On Mon, Jul 25, 2022 at 03:24:42AM +1000, Mark Jamsek wrote:
> > On 22-07-25 02:54am, Mark Jamsek wrote:
> > > I got the idea from stsp the other day when he mentioned that he doesn't
> > > always navigate diffs linearly, and I started paying close attention to
> > > how I read diffs. He's right--sometimes I jump around a fair bit too,
> > > and sometimes I read them top to bottom. In both cases, I find these key
> > > maps help navigating diffs.
> > > 
> > > Two new sets of key maps are proposed:
> > >   P - jump to the previous file
> > >   N - jump to the next file
> > >   { - jump to the previous hunk
> > >   } - jump to the next hunk
> > > 
> > 
> > Revised diff removes the conflicting N key map!
> > 
> > I'm not super happy with the () for prev/next file, but I can't think of
> > anything better right now.
> > 
> >  Two new sets of key maps are proposed:
> >    ( - jump to the previous file
> >    ) - jump to the next file
> >    { - jump to the previous hunk
> >    } - jump to the next hunk
> 
> In the past tog already did a secondary pass over the diff, reading
> over all lines with getline() to calculate line offsets. (See the
> function get_filestream_info() which was removed in
> commit fe621944e83fe6367f7bff97128b4240a9cdc7c5).
> For large diffs that are several metabytes in size this would slow down
> the loading phase of the diff view considerably. Commits which trigger
> this behaviour are those which update the clang compiler in the openbsd
> base system.

Thanks! I wasn't aware of this, but I'll take a look. And thanks to sdk,
I've got a 23MB/675KLOC diff I can test against too :)

> Ideally, the diff code would produce hunk-offsets in addition to
> the line offsets it is already producing, such that tog could
> simply jump to those already known offsets once the diff result
> has become available.

I think that would be a lot better! Tbh, my first instinct was that this
should be done in the diff code because we already know each line type
as the diff is produced, but I wasn't sure how the proposed key maps
would be received so wanted to get a feel first.

I'm happy to hack on the diff code. We could use a bitarray or even
build an index of each line as we save offsets so we'd have the type of
each line in the diff. Then we could do away with the regex for colours
as we'd have each meta/hunk/' '/+/- line indexed. What do you think? Or
would you prefer we just have another array for hunk offsets? I don't
mind either way.

> The diff code is managed in a separate repository, diff.git, intended
> to be generally reusable outside of Got. I can give you commit access
> there. The commit procedure we've been using is to commit changes to
> files which originate in diff.git to diff.git itself first, and then
> sync outstanding changes over to got.git in a single commit.

Sounds good! Thanks, Stefan. I'll clone the repo and start hacking :)

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68