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

Re: [PATCH v2 0/3] worktree: teach add to accept --reason with --lock

From
Eric Sunshine <sunshine@sunshineco.com>
Date
Jul 10, 2021, 07:15 UTC
Message-ID
<CAPig+cQyOgQb1zHA49kOT4+2SmTxUjZpfQ-f1ZxV1AwigCz01A@mail.gmail.com>
In-Reply-To
<xmqqv95j79ut.fsf@gitster.g>
On Fri, Jul 9, 2021 at 12:03 PM Junio C Hamano <gitster@pobox.com> wrote:
Show 18 quoted lines
> Eric Sunshine <sunshine@sunshineco.com> writes:
> > The reason I suggested re-purposing `add_opts.keep_locked` is to avoid
> > polluting that structure with members having overlapping meaning, thus
> > reducing the confusion-factor for clients of that structure (assuming
> > that a tri-state `keep_locked` is indeed less confusing). That doesn't
> > preclude adding a new variable or two local to the `add()` function to
> > facilitate keeping `add_opts` pure. For instance, it might be as
> > simple as the below patch.
>
> True.  It is less trivial to construct the command line option
> parser so that --reason=<why> and --lock can be given in any order
> (e.g. they no longer can be a simple OPT_BOOL and OPT_STRING that
> can be given independently but needs some postprocessing like your
> patch did), but it is not rocket science and keeping add_opts struct
> leaner will reduce the risk of runtime confusion, I would think, but
> at the same time, the room for runtime confusion would probably be
> minor to begin with---so I am fine, if the coder cannot cleanly
> write the option parser to do so, with the code as posted.

Although the leaner approach seems "simpler" and more obvious to me, it is subjective, and I can see how other people might find a tri-state add_opts::keep_locked confusing. So, I'm fine with it either way and don't consider it a blocker at all.

Previous: Junio C HamanoNext: Stephen Manz via GitGitGadget
Message 15 of 27 in “worktree: teach `add` to accept --reason <string> with --lock”
  1. worktree: teach `add` to accept --reason <string> with --lockStephen Manz via GitGitGadget, Jul 6, 2021
  2. Eric SunshineJul 6, 2021
  3. Junio C HamanoJul 6, 2021
  4. Eric SunshineJul 9, 2021
  5. Junio C HamanoJul 9, 2021
  6. Eric SunshineJul 10, 2021
  7. 0/3 worktree: teach add to accept --reason with --lockStephen Manz via GitGitGadget, Jul 8, 2021
  8. 1/3 t2400: remove unneeded `git rev-parse` from '"add" worktree with lock' testStephen Manz via GitGitGadget, Jul 8, 2021
  9. 2/3 worktree: default lock string should be marked with `_()` for translationStephen Manz via GitGitGadget, Jul 8, 2021
  10. Eric SunshineJul 9, 2021
  11. Junio C HamanoJul 9, 2021
  12. 3/3 worktree: teach `add` to accept --reason <string> with --lockStephen Manz via GitGitGadget, Jul 8, 2021
  13. Eric SunshineJul 9, 2021
  14. Junio C HamanoJul 9, 2021
  15. Eric SunshineJul 10, 2021
  16. 0/3 worktree: teach add to accept --reason with --lockStephen Manz via GitGitGadget, Jul 11, 2021
  17. 2/3 worktree: mark lock strings with `_()` for translationStephen Manz via GitGitGadget, Jul 11, 2021
  18. 1/3 t2400: clean up '"add" worktree with lock' testStephen Manz via GitGitGadget, Jul 11, 2021
  19. 3/3 worktree: teach `add` to accept --reason <string> with --lockStephen Manz via GitGitGadget, Jul 11, 2021
  20. Eric SunshineJul 13, 2021
  21. Eric SunshineJul 13, 2021
  22. 0/3 worktree: teach add to accept --reason with --lockStephen Manz via GitGitGadget, Jul 15, 2021
  23. 1/3 t2400: clean up '"add" worktree with lock' testStephen Manz via GitGitGadget, Jul 15, 2021
  24. 2/3 worktree: mark lock strings with `_()` for translationStephen Manz via GitGitGadget, Jul 15, 2021
  25. 3/3 worktree: teach `add` to accept --reason <string> with --lockStephen Manz via GitGitGadget, Jul 15, 2021
  26. Eric SunshineJul 15, 2021
  27. Eric SunshineJul 17, 2021

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.