Re: [PATCH v2 1/1] reset: support the --stdin option

2 messages, 2 authors, 2017-01-27 · open the first message on its own page

Re: [PATCH v2 1/1] reset: support the --stdin option

From: Junio C Hamano <hidden>
Date: 2017-01-27 18:33:20

Jeff King [off-list ref] writes:
A few minor suggestions:
quoted
+--stdin::
+	Instead of taking list of paths from the command line,
+	read list of paths from the standard input.  Paths are
+	separated by LF (i.e. one path per line) by default.
+
+-z::
+	Only meaningful with `--stdin`; paths are separated with
+	NUL character instead of LF.
Is it worth clarifying that these are paths, not pathspecs? The word
"paths" is used to refer to the pathspecs on the command-line elsewhere
in the document.
If the code forces literal pathspecs, then what the user feeds to
the command is no longer pathspecs from the user's point of view,
and I agree that the distinction is important.  

If the remainder of the documentation is loose in terminology and
the reason why we were able to get away with it was because we
consistently used "paths" when we meant "pathspec", these existing
mention of "paths" have to be tightened, either in this patch or a
preparatory step patch before this one, because the addition of
"this thing reads paths not pathspecs" is what makes them ambiguous.

Thanks.  I re-read the part of the code that reads and unquotes as
necessary in the patch and they looked correct.



Re: [PATCH v2 1/1] reset: support the --stdin option

From: Jeff King <hidden>
Date: 2017-01-27 22:19:10

On Fri, Jan 27, 2017 at 10:30:48AM -0800, Junio C Hamano wrote:
quoted
Is it worth clarifying that these are paths, not pathspecs? The word
"paths" is used to refer to the pathspecs on the command-line elsewhere
in the document.
If the code forces literal pathspecs, then what the user feeds to
the command is no longer pathspecs from the user's point of view,
and I agree that the distinction is important.  

If the remainder of the documentation is loose in terminology and
the reason why we were able to get away with it was because we
consistently used "paths" when we meant "pathspec", these existing
mention of "paths" have to be tightened, either in this patch or a
preparatory step patch before this one, because the addition of
"this thing reads paths not pathspecs" is what makes them ambiguous.
I think a lot of the documentation uses <paths> to refer to pathspecs
(e.g., git-log(1), git-diff(1), etc).  As long as we're consistent with
that convention, I don't think it's that big a deal.

This spot needs a specific mention because it violates the convention.

I don't know if the are other spots where it might be unclear, but I
think we're probably better to tighten those as they come up, rather
than switch to saying "<pathspecs>" everywhere.

That's outside the scope of this series, though, I would think.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help