El 16/12/2007, a las 20:43, Junio C Hamano escribió:
Wincent Colaiuta <win@xxxxxxxxxxx> writes:
diff --git a/Documentation/diff-options.txt b/Documentation/diff-
options.txt
index 9ecc1d7..54207f0 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -92,10 +92,10 @@ endif::git-format-patch[]
file gives the default to do so.
--check::
- Warn if changes introduce trailing whitespace
- or an indent that uses a space before a tab. Exits with
- non-zero status if problems are found. Not compatible with
- --exit-code.
+ Warn if changes introduce whitespace problems (such as
+ trailing whitespace). Configuration and per-path attributes
+ control what git classifies as a whitespace problem (see
+ gitlink:git-config[1] and gitlink:gitattributes[5]).
This is not quite right, is it? The command still exits with exit
code.
It is just that the calling process does not see it if you let it
spawn
the pager.
Yes, I know, but I thought that in the interests of brevity it would
be best not to clutter up the man page with that kind of detail. If a
user is interested in the exit code they will probably do a search of
the man page for "exit" and find the --exit-code section, which gives
them the answer they want. Adding a line like "Also, if you pass --no-
pager will exit with a non-zero exit code" doesn't really add much
other than clutter IMO.
Incidentally, and this is really tangential, I find the whole "--no-
pager" UI pretty horrendous, along with all the other switches parsed
by git.c. Basically, understanding these switches requires the user to
know internal implementation details that he/she should most
definitely not have to know.
For example, "git --no-pager diff --check" works as expected but "git
diff --no-pager --check" carps with "error: invalid option: --no-
pager". To understand why this is so, the user needs to know too many
things which should be internal implementation details:
- that "git" is just a wrapper tool that chooses a low level command
to run and runs it
- that there are some parameters that affect the way all the other
commands run and should be passed to "git" only, prior to the
"subcommand" part
- that those parameters can't be passed after the "subcommand"
specification
And seeing as we are actively encouraging users to always use the "git
diff" form rather than "git-diff", in a sense we are actively
encourage them to think of Git as a single tool rather than a
collection of commands, it seems like an ugly wart that we then have
to teach them that some parameters must go *between* the "git" and the
"diff" but not after. And of course, if you always use the "git-diff"
then there's no way to use the --no-pager switch at all; you instead
have to use an environment variable.
I've been thinking about ways to iron out these wrinkles and unify
things. Here's my thinking about a possible cause of action:
1. Factor out the code in git.c which handles the common arguments
into separate functions; at this stage it's just a refactoring and the
behaviour is unchanged, but those separate functions will later be
used from the builtins.
2. Teach the builtin commands to handle those common arguments by
using the common functions. For example, we teach "git diff" to
understand the --no-pager option by leveraging the functions we just
factored out in the previous step.
3. Now, when git.c sees an argument that would normally go before a
"subcommand" it no longer needs to handle it directly itself but can
instead pass it along to the subcommand. In other words, where it
formerly saw "git --no-pager diff args..." and handled the --no-pager
part itself, it can instead just pass "--no-pager args..." to the
builtin diff. This gives us backward compatibility with the old
format, but the new format (user types "git diff --no-pager args...")
will work also.
4. Update the docs: make a common-options.txt page which is included
in all other manual pages either listing the options explicitly or
directing users to the git(7) manpage (which should probably become
git(1) if we are talking about Git as a single tool) to learn about
them.
Benefits of this approach: we'd have a consistent UI which didn't
require too much knowledge of the internals of Git, and all of the
following would work:
git diff --no-pager args...
git-diff --no-pager args...
git --no-pager diff args...
Looking at the options currently parsed by git.c, I think most of them
would be very straightforward to support in the way described above:
--paginate
--no-pager
--git-dir
--work-tree
--bare
Existing special cases:
--help (calls "git help foo")
--version
Problematic:
-p (short form of --paginate: seeing as -p is already used to mean
other things by other commands, I think this should be deprecated as a
synonym for --paginate)
Tricky:
--exec-path (this is the only one which git.c *must* handle before
passing control to the "subcommand", so it actually has to look ahead
past the "subcommand" part in order to see it and act upon it.
Basically it would just have to look at all the arguments up to but
not including a "--" pathspec marker checking to see if --exec-path is
supplied)
Of course, the above plan will only work for builtins, not for
scripts. An additional step would be needed to enable scripts to
handle these options; perhaps teaching "git rev-parse" something...
Cheers,
Wincent
-
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
