Re: [PATCH 3/4] merge options: add a conflict style member

[Elijah Newren <newren@gmail.com>]

On Fri, Mar 8, 2024 at 8:48 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> phillip.wood123@gmail.com writes:
>
> > I agree it is confusing, Elijah renamed it from ll-merge.c relatively
> > recently 6723899932e (merge-ll: rename from ll-merge, 2023-05-16). It
> > looks like the idea was to group it with the other merge* files:
>
> That reasoning cuts both ways.  When you are only interested in the
> upper level API functions, being able to skip ll-anything as "low
> level details" is a powerful thing.  merge-ll & hash-ll separated
> far apart will make it impossible.

merge-ll is wildly different than every other *-ll.h file we have in
the tree; the latter set of files may be misnamed, for reasons other
than what you are suggesting here.  hash-ll, hex-ll, etc. exist due to
the main include file having some rarely used API that require more
#include statements, and most users of e.g. hex functions can get away
with just including hex-ll.h rather than the full hex.h.  Thus,
hex-ll.h is _not_ "low level details that you can skip", but "the
_primary_ data structures and functions".  It doesn't get the name
hex.h, though, because if we did that then the folks that need both
primary parts of the API and the occasional additional function would
need to have two hex-related includes.  Also, every function declared
within hex-ll.h is still defined within hex.c; there is no separate
hex-ll.c file, and the same is true of all the other *-ll.h files
other than merge-ll.h.  As such, there is absolutely no relation
between hex-ll.h, hash-ll.h, fsmonitor-ll.h, etc.  Using an ll- prefix
on those filenames thus makes no sense to me.  (It's not clear that
-ll even makes sense as a suffix for these files either, but it's not
clear what else to use instead.  If I recall correctly I originally
put forward "-basics" as a possible name suffix for these files, but
someone else suggested "-ll", and not having any better ideas or
strong opinions I just went with it.)

merge-ll is different in that it is actually a separate level of the
API, and there are both a merge-ll.h file and a merge-ll.c file.

I originally had proposed only adding the hex-ll.h, hash-ll.h,
fsmonitor-ll.h, etc. files, but you suggested that ll-merge should
either be renamed or that these new files should be
(https://lore.kernel.org/git/CABPp-BErrVUnuDjL73edDpmkKUvs6Ny6cYwvueXw1toB4JcF-Q@mail.gmail.com/).


Now, all that said, and assuming we were to go back to a world where
the other *-ll.h files don't exist (or are renamed independently with
a different suffix), I'm still not understanding why ll-merge makes
more sense to you and Phillip than merge-ll.  Could you explain more?
If you're only interested in the upper-level API functions, that
suggests you are already at the function level and looking within a
given file.  The low-level functions are already split out into a
separate file, so you just don't go looking at that separate file.
However, if you're interested in "where in this codebase do I find the
stuff related to merging", then you aren't in a file but in a
directory listing.  The first usecase gains no benefit from renaming
these files back to ll-merge.[ch], while the other usecase would have
been much improved in my experience from being named merge-ll.[ch].
Granted, it's a really minor point, which was why I never brought it
up until you suggested making all the other *-ll.h files and
ll-merge.[ch] consistent; since renaming the other *-ll.h files made
no sense at all to me, I went with renaming ll-merge.[ch] to
merge-ll.[ch].

There's probably some other angle to this that you two are viewing
this from that just isn't apparent to me.  Sorry for not seeing it
(yet).  Hopefully the above context is at least helpful, though.