On 2017-01-09 18:29, Junio C Hamano wrote:
Junio C Hamano <gitster@xxxxxxxxx> writes:Junio C Hamano <gitster@xxxxxxxxx> writes:I wonder if it makes more sense to always move to toplevel upfront and consistently use path from the toplevel, perhaps like the patchs/the patch/the attached patch/ I meant.does. The first hunk is what you wrote but only inside MERGE_RR block, and the second hunk deals with converting end-user supplied paths that are relative to the original relative to the top-level. The tweaking of $orderfile you have in the first hunk may have to be tightened mimicking the way how "eval ... --sq ... ; shift" is used in the second hunk to avoid confusion in case orderfile specified by the end user happens to be the same as a valid revname (e.g. "master").And here is a squash-able patch to illustrate what I mean.By the way, I didn't think this through, but how is the orderfile that comes from the configuration file handled when it is not an absolute path?
Good question; it does whatever 'git diff' does, and that case isn't documented. It's also unclear if the globs are like the .gitignore patterns.
I would expect it to act like $GIT_DIR/info/exclude, but I haven't checked.
I think it is _wrong_ to take it as relative to where the user started the program.
Agreed.
The -O<file> parameter from the command line, when <file> is not absolute, should be taken as relative to where the user _thinks_ s/he is,
Agreed.
but when it comes from the diff.orderfile configuration and it is not absolute, it should be taken as relative to the top of the working tree.
Agreed.
As we always cd_to_top with the suggested SQUASH, it means that the orderfile that came from the configuration does not have to be touched, while the orderfile given via -O<file> on the command line needs prefixing.
Yes. By unconditionally running cd_to_top we should get the behavior we want even if 'git diff' uses the current working directory rather than the top-level directory.
I'll poke at 'git diff' to see what it does. -Richard
<<attachment: smime.p7s>>