From: Victoria Dye <vdye@github.com>
Date: Mon, 17 Jun 2024 19:23:39 GMT
Subject: Re: [PATCH 14/16] mktree: optionally add to an existing tree
Message-ID: <55e06e1b-580e-45fe-a914-d1ef252d5981@github.com>
In-Reply-To: <ZmltKHI-Vz1L44r8@tanuki>

Patrick Steinhardt wrote:
> On Tue, Jun 11, 2024 at 06:24:46PM +0000, Victoria Dye via GitGitGadget wrote:
>> diff --git a/Documentation/git-mktree.txt b/Documentation/git-mktree.txt
>> index afbc846d077..99abd3c31a6 100644
>> --- a/Documentation/git-mktree.txt
>> +++ b/Documentation/git-mktree.txt
>> @@ -40,6 +40,11 @@ OPTIONS
>>  	optional.  Note - if the `-z` option is used, lines are terminated
>>  	with NUL.
>>  
>> +<tree-ish>::
>> +	If provided, the tree entries provided in stdin are added to this tree
>> +	rather than a new empty one, replacing existing entries with identical
>> +	names. Not compatible with `--literally`.
> 
> I think it'd be a bit more intuitive is this was an option, like
> `--base-tree=` or just `--base=`.

To me, the positional '<tree-ish>' is more intuitive; it's reminiscent of
'read-tree' (but with '--empty' being the default, since there's no
equivalent to the existing index to overwrite). I consider 'read-tree'
relevant in this case because the updated 'mktree' allows a users to create
trees like:

$ git read-tree <tree-ish>
$ git update-index <entries
$ git write-tree

without the intermediate on-disk index. Conversely, there isn't really an
equivalent option to base the name on ('--base' is a bit overloaded, as it
typically refers to a merge/diff base), and I'd like to avoid adding more
potentially-confusing names to the overall Git UX if I can help it (even if
this is a plumbing command).

However, looking at other command documentation, I should at least drop
'[--]' from the usage string. While that is a separator used to signify "end
of options" using 'parse_options()', it's typically only included in the
usage string to separate different sets of positional arguments (e.g.
revisions from pathspecs). 

> 
> One question that comes up naturally in this context: when I have a base
> tree, how do I remove entries from it?

In patch 16 [1], entries with mode "0" are removed from the tree (similar to
'update-index').

[1] https://lore.kernel.org/git/a90d6d0c943283e9e7bd181cd6e9bb6d4572aaeb.1718130288.git.gitgitgadget@gmail.com/

> 
> Patrick