git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Elijah Newren <newren@gmail.com>
Date
Mar 9, 2024, 04:33 UTC
Message-ID
<CABPp-BFBNDkdL5Z-VOrv2Z3aWaA3OpWFqd6ZGmU9YC8jBRsAog@mail.gmail.com>
In-Reply-To
<xmqqwmqcfz4y.fsf@gitster.g>
On Fri, Mar 8, 2024 at 8:48 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 11 quoted lines
>
> 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.

Previous: Junio C HamanoNext: Junio C Hamano
Message 8 of 29 in “checkout: cleanup --conflict=”
  1. 0/4 checkout: cleanup --conflict=Phillip Wood via GitGitGadget, Mar 8, 2024
  2. 1/4 xdiff-interface: refactor parsing of merge.conflictstylePhillip Wood via GitGitGadget, Mar 8, 2024
  3. 2/4 merge-ll: introduce LL_MERGE_OPTIONS_INITPhillip Wood via GitGitGadget, Mar 8, 2024
  4. 3/4 merge options: add a conflict style memberPhillip Wood via GitGitGadget, Mar 8, 2024
  5. Junio C HamanoMar 8, 2024
  6. phillip.wood123@gmail.comMar 8, 2024
  7. Junio C HamanoMar 8, 2024
  8. Elijah NewrenMar 9, 2024
  9. Junio C HamanoMar 9, 2024
  10. 4/4 checkout: cleanup --conflict=<style> parsingPhillip Wood via GitGitGadget, Mar 8, 2024
  11. Junio C HamanoMar 8, 2024
  12. phillip.wood123@gmail.comMar 8, 2024
  13. Junio C HamanoMar 8, 2024
  14. phillip.wood123@gmail.comMar 11, 2024
  15. Junio C HamanoMar 11, 2024
  16. Phillip WoodMar 12, 2024
  17. Junio C HamanoMar 8, 2024
  18. phillip.wood123@gmail.comMar 8, 2024
  19. 0/5 checkout: cleanup --conflict=Phillip Wood via GitGitGadget, Mar 14, 2024
  20. 1/5 xdiff-interface: refactor parsing of merge.conflictstylePhillip Wood via GitGitGadget, Mar 14, 2024
  21. Junio C HamanoMar 14, 2024
  22. 2/5 merge-ll: introduce LL_MERGE_OPTIONS_INITPhillip Wood via GitGitGadget, Mar 14, 2024
  23. 3/5 merge options: add a conflict style memberPhillip Wood via GitGitGadget, Mar 14, 2024
  24. Junio C HamanoMar 14, 2024
  25. 4/5 checkout: cleanup --conflict=<style> parsingPhillip Wood via GitGitGadget, Mar 14, 2024
  26. Junio C HamanoMar 14, 2024
  27. 5/5 checkout: fix interaction between --conflict and --mergePhillip Wood via GitGitGadget, Mar 14, 2024
  28. Junio C HamanoMar 14, 2024
  29. Phillip WoodMar 14, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.