David Aguilar <davvid@xxxxxxxxx> writes: > On Mon, Mar 19, 2012 at 2:00 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote: >> Thanks. >> >> I do not know a difftool user, and I wasn't paying close attention to the >> discussion but I recall these points raised and I do not recall the >> resolutions: >> >> * In [1/9], use of pass_through would mean 'git difftool' must be fed the >> options for difftool and then options meant for underlying diff >> machinery. Is this limitation still there? If so, isn't this a >> regression? Shouldn't it at least be advertised to the users a lot >> stronger in documentation? > > Tim asserted that this is not the case. The options should pass > through. Hopefully there aren't any behavior changes between perl > versions and this option. > > I sent a patch adding a test case to cover this scenario. I would > prefer that we avoid a regression. If there's a regression then we > can do without Getopt::Long, IMO. Yeah. Does tonight's 'pu' pass for you? > I do not have Windows to test with, but this supposedly works since > Git.pm does not use git.exe either. Yeah, that matches my guess. >> * In [7/9], difftool--helper declares SUBDIRECTORY_OK, but there doesn't >> seem to be any inclusion of git-sh-setup in this script, and the patch >> does not have any effort to prepend $prefix to paths relative to $cwd. >> What good does the variable do here? > > I'll defer to Tim on this one. This seems like an oversight. It > seems like something should be done to handle it. Well, it does not seem to dot-source git-sh-setup so it probably stays where it was launched, so in that case there is nothing that needs to be done, including SUBDIRECTORY_OK which nobody would look at IIRC. > Also.. I think we need some tests to cover the new behavior. A test > to cover the subdirectory behavior would be especially helpful given > the note about [7/9]. Yeah, that makes sense. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html