On Mon, Dec 14, 2009 at 09:56:28PM -0800, Junio C Hamano wrote: > Let's try to do a bit more work to make the coverage complete. After > scanning "git grep -e start_async -e run_command" output, here is what I > came up with: > > - editor.c::launch_editor() that allows a custom editor named via > GIT_EDITOR does seem to honor your command line arguments. > > - pager.c::setup_pager() is used for GIT_PAGER and it does honor your > command line arguments. > > - ll-merge.c::ll_ext_merge() that is used to handle custom merge drivers > lets the user specify command line via templating to replace %O %A %B > and naturally it needs to be aware of the command line arguments. I think it is important to note in user-facing documentation (like release notes describing the proposed textconv change) that these are not just "honor your command line arguments" but "execute your command with /bin/sh". Maybe that is obviously how it is implemented, but I think it should be made clear that you have the power to use (and responsibility to protect yourself from) arbitrary shell code. > - diff.c::run_external_diff() that runs GIT_EXTERNAL_DIFF defines that > the command has to take 7 parameters in a fixed order, and is not > designed to permute its arguments like ll_ext_merge() does, but these > days people don't use it directly (they use it indirectly via > "difftool" wrapper), so it probably is not an issue. There is also diff.*.command, which I think people _do_ set manually (I used to, until I wrote textconv. :) ). It does not use the shell currently. I agree that people almost certainly have to write a shell-script wrapper anyway. But I wonder if we should pass it through the shell, just for the sake of consistency with the other variables (in particular, if textconv changes, I think diff.*.command is closely related). -Peff -- 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