On 2011-10-30 07:09, Junio C Hamano wrote:
I do not think any of our scripted Porcelains use "set -e"; rather they try to catch errors and produce messages that are more appropriate for specific error sites.
git-dirdiff being a wrapper script, I reasoned that the underlaying command would already have printed a sufficient error message, so what was left was just to exit. But you're right in that some of the commands will not give sufficient context. I'll put in some more detailed error handling.
I do not think any of our scripted Porcelains use "mktemp" especially "mktemp -d". How portable is it?
Even if it is not part of Posix, I reckon since it is a part of the coreutils, it is available in most GNU userlands, inclusive GnuWin32. I don't have any live BSD systems available, but based on the man pages it seems to be available there too, albeit with some different options. --tmpdir seems to be more of a problem in than respect than -d. I'll see if I can find a set of options that are digestable to most platforms. I think it is unfortunate to use the current directory as scratch space; it can be a network disk, or even read-only. mktemp can in contrast make a directory in a place which is not spilled to disk.
It is not clear to me why you need to grab the list of paths in toc and iterate over them one by one. IOW, why isn't this sufficient?
This design decision was probably appropriate in an earlier iteration, but as you point out, it is indeed superfluous now! I apologize for not realizing that myself.
suspect is true), we would probably want to make it an option to "git diff" itself, and not a separate git subcommand like "dirdiff". I can see
Although being an able *user* of Git, I am not (yet) familiar enough with the codebase to have a patch ready for `git diff` itself. It would certainly require more rigorous testing.
"git diff" (and possibly even things like "git log -p") populating two temporary directories (your old/ and new/) and running a custom program specified by GIT_EXTERNAL_TREEDIFF environment variable, instead of doing
Would it be better to have yet another configuration available for this option instead of reusing the existing infrastructure for `git difftool`?
It also is not clear what could be used in "$@". Obviously you would not want things like "-U20" and "--stat" fed to the first "list of paths" phase, but there may be some options you may want to give to the inner "external diff" thing.
Ideally, it should work the same way as `git difftool`.
For example, how well does this approach work with -M/-C (I do not think it would do anything useful, but I haven't thought things through). It would be nice if we gave the hint to the external
As of now, files that are moved will turn up a different places in the tree without any annotations, and off the top of my head I don't see any obvious way to propagate such hints. If the diff tool is sophisticated can probably use the same heuristics as Git currently does, but I am unaware of any that is able to do that yet.
I wouldn't mind carrying a polished version of this in contrib/ for a cycle or two in order to let people try it out and get kinks out of its design.
I would appreciate that! I'll submit a reworked proposal taking you comments into account. It may take a few days due to unrelated engagements, though. -- Roland. -- 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