Add `--pathspec-from-file` option #445

SyntevoAlex · 2019-11-04T19:02:40Z

Changes from V3:
----------------
The branch was rebased onto latest `master`.
These patches remain unchanged since they were accepted in V3:
	parse-options.h: add new options `--pathspec-from-file`, `--pathspec-file-nul`
	pathspec: add new function to parse file
	doc: reset: synchronize <pathspec> description
	reset: support the `--pathspec-from-file` option
	doc: commit: synchronize <pathspec> description
	commit: support the --pathspec-from-file option
These patches are new, extending support to more git commands:
	cmd_add: prepare for next patch
	add: support the --pathspec-from-file option
	doc: checkout: remove duplicate synopsis
	doc: checkout: fix broken text reference
	doc: checkout: synchronize <pathspec> description
	doc: restore: synchronize <pathspec> description
	checkout, restore: support the --pathspec-from-file option

Cc: Junio C Hamano [email protected]
Cc: Phillip Wood [email protected]

SyntevoAlex · 2019-11-04T19:25:50Z

/submit

gitgitgadget · 2019-11-04T19:27:04Z

Submitted as [email protected]

pathspec.c

Documentation/git-reset.txt

Documentation/git-commit.txt

Documentation/git-reset.txt

gitgitgadget · 2019-11-06T04:51:25Z

Documentation/git-commit.txt

@@ -13,7 +13,7 @@ SYNOPSIS
 	   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]


On the Git mailing list, Junio C Hamano wrote (reply to this):

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]> writes: > From: Alexandr Miloslavskiy <[email protected]> > > Synchronize it to `git add`, which has a pretty good description. > This also better disambiguates <file>... header. "When files are given on..." is no longer true with this change (it wasn't true with the code before this change anyway). When pathspec is given on the command line, commit the contents of the files that match the pathspec without recording the changes already added to the index. ... The second sentence also says "these files", but that can be left as-is, since it would refer to "the files that match ..." explained in the above sentence. > +For more details about the <pathspec> syntax, see the 'pathspec' entry > +in linkgit:gitglossary[7]. I am not sure if we want to repeat this all over the place. We do not say "For details about the <commit> syntax, see the 'SPECIFYING REVISIONS' section of linkgit:git-rev-parse[1]" for every command that takes <commit> from the command line. Is your urge to suggest adding this sentence coming from that you are much more familiar with <commit> than <pathspec>? In other words, if you were more familiar with Git, would you still be adding this (and not corresponding one for <commit>)?

On the Git mailing list, Alexandr Miloslavskiy wrote (reply to this):

I think I have implemented most suggestions in PatchV2. Thanks! > I am not sure if we want to repeat this all over the place. > > We do not say "For details about the <commit> syntax, see the > 'SPECIFYING REVISIONS' section of linkgit:git-rev-parse[1]" for > every command that takes <commit> from the command line. > > Is your urge to suggest adding this sentence coming from that you > are much more familiar with <commit> than <pathspec>? In other > words, if you were more familiar with Git, would you still be adding > this (and not corresponding one for <commit>)? Commit is a well known term, dating from ancient times like CVS or even older. Pathspec, however, is something new. When I pretend to be someone new to git, I see it this way: 1) Let's read "git commit" documentation 2) Where on this commandline do I put my filename?! So yes, I would repeat it in every location that could be popular for new users.

On the Git mailing list, Junio C Hamano wrote (reply to this):

Alexandr Miloslavskiy <[email protected]> writes: > I think I have implemented most suggestions in PatchV2. Thanks! > >> I am not sure if we want to repeat this all over the place. >> >> We do not say "For details about the <commit> syntax, see the >> 'SPECIFYING REVISIONS' section of linkgit:git-rev-parse[1]" for >> every command that takes <commit> from the command line. >> >> Is your urge to suggest adding this sentence coming from that you >> are much more familiar with <commit> than <pathspec>? In other >> words, if you were more familiar with Git, would you still be adding >> this (and not corresponding one for <commit>)? > > Commit is a well known term, dating from ancient times like CVS or > even older. That's more or less irrelevant. I am reacting to this from your change that you omitted quoting in your reply: > +For more details about the <pathspec> syntax, see the 'pathspec' entry > +in linkgit:gitglossary[7]. That sentence is for those who have some notion of <pathspec> but does not know enough about its syntax. CVS does not let you specify <commit> like "master^{/^fix frotz}~4"; A user a user who is familiar with CVS's commits would still want to refer to the section "For details about the <commit> syntax". I am not advocating to add the reference to SPECIFYING REVISIONS section; instead we should let readers know that every time they see <defined word>, they can refer to the glossary for more details. > Pathspec, however, is something new. Compared to CVS, everything in Git may be new, but that was a news in 2010, not this year.

On the Git mailing list, Alexandr Miloslavskiy wrote (reply to this):

On 07.11.2019 6:54, Junio C Hamano wrote: > I am reacting to this from your change that you omitted quoting in > your reply: > >> +For more details about the <pathspec> syntax, see the 'pathspec' entry >> +in linkgit:gitglossary[7]. > > That sentence is for those who have some notion of <pathspec> but > does not know enough about its syntax. In the perfect world, I would expect _every_ 'pathspec' word in text to be an HTML-style link to a dedicated article, not just a paragraph in glossary. MSDN is in general a good example [1]: there, it's easy to read only a small portion of article, ignoring anything you're not interested in, and still have all links at hand. Regarding dedicated page: the content of 'pathspec' in glossary is already long enough for a page, and it could benefit from additional examples. Also, having a dedicated page makes linking easier, so that user doesn't have to scroll glossary. Regarding links: I don't really understand what git's doc format allows. Is it just pure text in worst (or even average) case? If it's usually something with clickable links, it could be worth to just insert links everywhere. If it's usually plaintext, then "see the 'pathspec' entry in linkgit:gitglossary[7]" is a bit too verbose to repeat on every occasion. Still, I'd like to see links everywhere. One big reason is to let reader know that the explanation actually exists! A compromise solution is to give every article header like this: This article uses the following terms which are explained in linkgit:gitglossary[7]: * pathspec * tree-ish * refspec If it's close to top of article, then chances are that everyone will notice it. Also, it will not require extra verbosity in plaintext. > CVS does not let you specify <commit> like "master^{/^fix frotz}~4"; > A user a user who is familiar with CVS's commits would still want to > refer to the section "For details about the <commit> syntax". > > I am not advocating to add the reference to SPECIFYING REVISIONS > section; instead we should let readers know that every time they see > <defined word>, they can refer to the glossary for more details. I now think that my arguments apply to <pathspec> AND <commit> in the same way. Indeed a user can't know complex <commit> variants until he/she reads it in git docs. ---- [1] https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea

Documentation/git-commit.txt

SyntevoAlex · 2019-11-06T15:50:25Z

/submit

gitgitgadget · 2019-11-06T15:51:39Z

Submitted as [email protected]

gitgitgadget · 2019-11-07T11:15:44Z

This branch is now known as am/pathspec-from-file.

gitgitgadget · 2019-11-07T11:15:46Z

This patch series was integrated into pu via git@13376d9.

gitgitgadget · 2019-11-08T08:49:04Z

This patch series was integrated into pu via git@66797f9.

gitgitgadget · 2019-11-10T08:59:45Z

This patch series was integrated into pu via git@f5ab9af.

gitgitgadget · 2019-11-11T04:51:48Z

This patch series was integrated into pu via git@5ad7d3f.

gitgitgadget · 2019-11-11T11:50:00Z

This patch series was integrated into pu via git@7126fe7.

gitgitgadget · 2019-11-12T08:45:31Z

This patch series was integrated into pu via git@eb1041e.

gitgitgadget · 2019-11-13T07:17:42Z

This patch series was integrated into pu via git@df7e54e.

gitgitgadget · 2019-11-13T11:01:56Z

This patch series was integrated into pu via git@b8a68d8.

gitgitgadget · 2019-11-14T03:36:41Z

This patch series was integrated into pu via git@cfdb1c8.

gitgitgadget · 2019-11-15T06:50:02Z

This patch series was integrated into pu via git@a096466.

gitgitgadget · 2019-11-18T06:31:50Z

This patch series was integrated into pu via git@49ff8fa.

gitgitgadget · 2019-11-19T05:21:33Z

This patch series was integrated into pu via git@3618ed0.

pathspec.c

Documentation/git-reset.txt

`git add` shows an example of good writing, follow it. Signed-off-by: Alexandr Miloslavskiy <[email protected]>

Decisions taken for simplicity: 1) For now, `--pathspec-from-file` is declared incompatible with `--patch`, even when <file> is not `stdin`. Such use case it not really expected. 2) It is not allowed to pass pathspec in both args and file. `you must specify path(s) to restore` block was moved down to be able to test for `pathspec.nr` instead, because testing for `argc` is no longer correct. `git switch` does not support the new options because it doesn't expect `<pathspec>` arguments. Signed-off-by: Alexandr Miloslavskiy <[email protected]>

SyntevoAlex · 2019-12-03T14:01:40Z

/submit

gitgitgadget · 2019-12-03T14:02:39Z

Submitted as [email protected]

SyntevoAlex · 2019-12-03T15:23:31Z

/preview

gitgitgadget · 2019-12-03T15:24:24Z

Preview email sent as [email protected]

SyntevoAlex · 2019-12-03T15:43:04Z

/preview

gitgitgadget · 2019-12-03T15:44:04Z

Preview email sent as [email protected]

SyntevoAlex · 2019-12-03T15:47:08Z

/preview

gitgitgadget · 2019-12-03T15:47:56Z

Preview email sent as [email protected]

SyntevoAlex · 2019-12-03T16:31:14Z

/preview

gitgitgadget · 2019-12-03T16:32:04Z

Preview email sent as [email protected]

gitgitgadget · 2019-12-03T17:00:43Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

"Alexandr Miloslavskiy via GitGitGadget" <[email protected]>
writes:

> Changes from V3:

Yikes, perhaps our mails crossed or something?  I think the previous
round is already in 'next'.

Let's wait and see they cook enough to graduate to 'master', and
build a separate series on top to teach other commands the option
using the facility introduced by the current series (which is the
first 6 patches you sent here).

Thanks.

gitgitgadget · 2019-12-03T17:09:38Z

On the Git mailing list, Alexandr Miloslavskiy wrote (reply to this):

On 03.12.2019 17:55, Junio C Hamano wrote:

>> Changes from V3:
> 
> Yikes, perhaps our mails crossed or something?  I think the previous
> round is already in 'next'.
> 
> Let's wait and see they cook enough to graduate to 'master', and
> build a separate series on top to teach other commands the option
> using the facility introduced by the current series (which is the
> first 6 patches you sent here).

My intent is to support more commands, so I was working on other patches 
in the background. Today more patches were ready and I wasn't sure 
whether to submit another topic or continue the old one. After some 
thinking, I decided to continue the old one.

Apparently, I guessed wrong :) Roger: I will wait until the first part 
graduates to 'master', then submit more patches.

Please give me an advice: when the time comes, shall I prepare even more 
patches and submit a massive branch, or shall I submit today's remaining 
patches, then wait again? I imagine that massive branches are scary and 
will deter reviewers?

gitgitgadget · 2019-12-04T19:28:10Z

On the Git mailing list, Junio C Hamano wrote (reply to this):

Alexandr Miloslavskiy <[email protected]> writes:

> On 03.12.2019 17:55, Junio C Hamano wrote:
>
>>> Changes from V3:
>>
>> Yikes, perhaps our mails crossed or something?  I think the previous
>> round is already in 'next'.
>>
>> Let's wait and see they cook enough to graduate to 'master', and
>> build a separate series on top to teach other commands the option
>> using the facility introduced by the current series (which is the
>> first 6 patches you sent here).
>
> My intent is to support more commands, so I was working on other
> patches in the background. Today more patches were ready and I wasn't
> sure whether to submit another topic or continue the old one. After
> some thinking, I decided to continue the old one.

Well I've split the new patches into its own topic to queue on the
am/pathspec-f-f-checkout branch, that builds directly on top of the
am/pathspec-from-file branch, for now.  I suspect that they may want
to be two topics (i.e. for "add" and for "checkout/restore"), but
I'd like to keep them out of 'next' either way for a while until the
base topic proves to be solid enough.

> Please give me an advice: when the time comes, shall I prepare even
> more patches and submit a massive branch, or shall I submit today's
> remaining patches, then wait again? I imagine that massive branches
> are scary and will deter reviewers?

Scary will probably not be an issue for the follow-up topics around
the pathspec-from-file theme, but a huge topic tends to wear out the
author and the reviewers, inviting trivial bugs that would otherwise
be found easily go unnoticed.

Thanks.

gitgitgadget · 2019-12-05T10:55:34Z

On the Git mailing list, Alexandr Miloslavskiy wrote (reply to this):

On 04.12.2019 20:25, Junio C Hamano wrote:

> Scary will probably not be an issue for the follow-up topics around
> the pathspec-from-file theme, but a huge topic tends to wear out the
> author and the reviewers, inviting trivial bugs that would otherwise
> be found easily go unnoticed.

OK, thanks! I will pause for now and wait until existing patches are in 
`master`.

gitgitgadget · 2019-12-05T22:42:32Z

This patch series was integrated into pu via git@0effd37.

gitgitgadget · 2019-12-05T22:54:55Z

This branch is now known as am/pathspec-f-f-checkout.

gitgitgadget · 2019-12-06T19:54:34Z

This patch series was integrated into pu via git@d41dec3.

gitgitgadget · 2019-12-06T22:30:11Z

This patch series was integrated into pu via git@57a8519.

gitgitgadget · 2019-12-10T05:55:54Z

This patch series was integrated into pu via git@d288834.

gitgitgadget · 2019-12-10T18:08:20Z

This patch series was integrated into pu via git@64df397.

gitgitgadget · 2019-12-10T23:52:46Z

This patch series was integrated into pu via git@d96bb8b.

gitgitgadget · 2019-12-10T23:52:47Z

This patch series was integrated into next via git@6b018bd.

SyntevoAlex · 2019-12-11T10:12:17Z

Closing this PR: first part was merged into master now (see git @ c58ae96).
As discussed with Junio, I will submit another PR with second batch of patches.

gitgitgadget bot reviewed Nov 5, 2019

View reviewed changes

pathspec.c Show resolved Hide resolved

gitgitgadget bot reviewed Nov 5, 2019

View reviewed changes

Documentation/git-reset.txt Show resolved Hide resolved

gitgitgadget bot reviewed Nov 5, 2019

View reviewed changes

Documentation/git-reset.txt Show resolved Hide resolved

gitgitgadget bot reviewed Nov 5, 2019

View reviewed changes

Documentation/git-commit.txt Show resolved Hide resolved

gitgitgadget bot reviewed Nov 6, 2019

View reviewed changes

Documentation/git-reset.txt Show resolved Hide resolved

gitgitgadget bot reviewed Nov 6, 2019

View reviewed changes

Documentation/git-reset.txt Show resolved Hide resolved

gitgitgadget bot reviewed Nov 6, 2019

View reviewed changes

Documentation/git-commit.txt Show resolved Hide resolved

SyntevoAlex force-pushed the #0207_pathspec_from_file branch from f484704 to cb5fc9b Compare November 6, 2019 15:25

gitgitgadget bot added the pu label Nov 7, 2019

SyntevoAlex mentioned this pull request Nov 7, 2019

Teach git reset to optionally read the pathspecs from stdin #133

Closed

gitgitgadget bot reviewed Nov 19, 2019

View reviewed changes

pathspec.c Show resolved Hide resolved

gitgitgadget bot reviewed Nov 19, 2019

View reviewed changes

Documentation/git-reset.txt Show resolved Hide resolved

SyntevoAlex added 3 commits December 3, 2019 14:35

doc: checkout: synchronize <pathspec> description

fd16675

`git add` shows an example of good writing, follow it. Signed-off-by: Alexandr Miloslavskiy <[email protected]>

doc: restore: synchronize <pathspec> description

9e37a74

`git add` shows an example of good writing, follow it. Signed-off-by: Alexandr Miloslavskiy <[email protected]>

SyntevoAlex force-pushed the #0207_pathspec_from_file branch from dacf32b to c4dd4ea Compare December 3, 2019 13:35

SyntevoAlex changed the title ~~Add --pathspec-from-file option for reset, commit~~ Add --pathspec-from-file option Dec 3, 2019

SyntevoAlex mentioned this pull request Dec 4, 2019

Fix Cc: footer handling gitgitgadget/gitgitgadget#163

Merged

SyntevoAlex closed this Dec 11, 2019

		@@ -13,7 +13,7 @@ SYNOPSIS
		[-F <file> \| -m <msg>] [--reset-author] [--allow-empty]

Add --pathspec-from-file option #445

Add --pathspec-from-file option #445

Uh oh!

Conversation

SyntevoAlex commented Nov 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SyntevoAlex commented Nov 4, 2019

Uh oh!

gitgitgadget bot commented Nov 4, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gitgitgadget bot Nov 6, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Nov 6, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

gitgitgadget bot Nov 7, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SyntevoAlex commented Nov 6, 2019

Uh oh!

gitgitgadget bot commented Nov 6, 2019

Uh oh!

gitgitgadget bot commented Nov 7, 2019

Uh oh!

gitgitgadget bot commented Nov 7, 2019

Uh oh!

gitgitgadget bot commented Nov 8, 2019

Uh oh!

gitgitgadget bot commented Nov 10, 2019

Uh oh!

gitgitgadget bot commented Nov 11, 2019

Uh oh!

gitgitgadget bot commented Nov 11, 2019

Uh oh!

gitgitgadget bot commented Nov 12, 2019

Uh oh!

gitgitgadget bot commented Nov 13, 2019

Uh oh!

gitgitgadget bot commented Nov 13, 2019

Uh oh!

gitgitgadget bot commented Nov 14, 2019

Uh oh!

gitgitgadget bot commented Nov 15, 2019

Uh oh!

gitgitgadget bot commented Nov 18, 2019

Uh oh!

gitgitgadget bot commented Nov 19, 2019

Uh oh!

Uh oh!

Uh oh!

SyntevoAlex commented Dec 3, 2019

Uh oh!

gitgitgadget bot commented Dec 3, 2019

Uh oh!

SyntevoAlex commented Dec 3, 2019

Uh oh!

gitgitgadget bot commented Dec 3, 2019

Uh oh!

SyntevoAlex commented Dec 3, 2019

Uh oh!

gitgitgadget bot commented Dec 3, 2019

Uh oh!

SyntevoAlex commented Dec 3, 2019

Uh oh!

gitgitgadget bot commented Dec 3, 2019

Uh oh!

SyntevoAlex commented Dec 3, 2019

Uh oh!

Add `--pathspec-from-file` option #445

Add `--pathspec-from-file` option #445

SyntevoAlex commented Nov 4, 2019 •

edited

Loading