From: Matheus Tavares Bernardino Date: Fri, 17 Apr 2020 21:19:07 GMT Subject: Re: [PATCH] grep: follow conventions for printing paths w/ unusual chars Message-ID: In-Reply-To: On Fri, Apr 17, 2020 at 3:45 AM Junio C Hamano wrote: > > Matheus Tavares writes: > > > + if (opt->relative && opt->prefix_length) > > + quote_path_relative(filename + tree_name_len, opt->prefix, out); > > + else > > + quote_c_style(filename + tree_name_len, out, NULL, 0); > > Yup. This solves the discrepancy reported correctly (i.e. both > sides should do the quoting, the original only quoted when relative, > and the new code corrects the other side). > > > + if (tree_name_len) > > + strbuf_insert(out, 0, filename, tree_name_len); > > I am not quite sure about this part, though. > > Earlier we inserted the latter part of filename (after offset > tree_name_len) to strbuf "out" after quoting, and then we are > prefixing the earlier part of the filename without quoting to that > same "out". Wouldn't a path ABCDEF (I do not literally mean that > these ascii alphabets need quoting---just imagine each of these > stands for a different letter and some causes the path to be quoted) > with tree_name_len pointing somewhere in the middle be added as (an > analog of) ABC"DEF", i.e. literal prefix with remainder quoted? Right. But the ABC prefix here is always a tree name, so the output is one of the following: :"unusual path":line :"unusual path":line "unusual path":line (Always having the entire "unusual path" inside the double quotes, though) > I would (perhaps näively) expect that the whole thing would be placed > inside a dq pair in such a case, even if the prefix part alone would > not require quoting. Hm, it might be just me, but I think that including the tree name and path in the same dq pair could be a bit confusing, as they can seem like one unique thing. There is also the case of ref names containing unusual chars, as dq's. Such ref names are not currently escaped in the output of other commands as checkout, branch or describe. But if we include the ref name in grep's output quote, it would be escaped, which would be a minor inconsistency (I don't know if that's relevant, though).