* Re: [RFH] convert shortlog to use parse_options
From: Pierre Habouzit @ 2007-12-13 9:35 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20071213091055.GA5674@coredump.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 1330 bytes --]
On Thu, Dec 13, 2007 at 09:10:56AM +0000, Jeff King wrote:
> On Thu, Dec 13, 2007 at 10:06:04AM +0100, Pierre Habouzit wrote:
>
> > No we can't. And I believe that such a thing is definitely bad practice
> > :/ So if you really need to, we will have to add some PARSE_OPT_STICKARG
> > or sth alike that would check that the argument was "sticked" to the
> > option either with `-wA,B,C` or `--long-opt=A,B,C` depending on the fact
> > that an option is short or long.
>
> Yes, I am not sure if the right solution is to just say "we are changing
> how -w works". Because it either must change, or it must be inconsistent
> with the rest of the option parsing for the rest of eternity.
In fact we have kind of the issue for every single optional argument out
there:
$ git describe --abbrev HEAD
error: option `abbrev' expects a numerical value
[...]
*ouch*
So I believe that with optional arguments we must change the way we do
things, and that we _must_ enforce the argument to be sticked in that
case. Because this kind of backward incompatibility I totally missed in
the first place is unacceptable. Patch on its way.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* Re: [PATCH] clone: support cloning full bundles
From: Santi Béjar @ 2007-12-13 9:37 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Git Mailing List
In-Reply-To: <Pine.LNX.4.64.0712121846070.27959@racer.site>
On Dec 12, 2007 7:46 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Wed, 12 Dec 2007, Santi B?jar wrote:
>
> > On Dec 12, 2007 3:50 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> >
> > > On Wed, 12 Dec 2007, Santi B?jar wrote:
> > >
> > > > It still fails for incremental bundles.
> > >
> > > Of course it does. The whole point of incremental bundles is that
> > > they do _not_ contain all objects, but rely on some objects being
> > > present on the "fetch" side.
> >
> > I know this. But then there is no bundle equivalent of the shallow
> > clones, as with:
> >
> > git clone --depth <depth> <repo>
>
> Ah, thanks. Completely forgot about this.
But you were right, I confused incremental and shadow. So the next step would be
git bundle create --depth <depth> ...
I'll see what I can do.
Thanks
^ permalink raw reply
* Re: [PATCH] Teach git-gui to split hunks
From: Johannes Sixt @ 2007-12-13 9:41 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, Jason Sewall, Shawn O. Pearce, David,
Marco Costalba, Andy Parkins, git
In-Reply-To: <7vhcin3rv4.fsf@gitster.siamese.dyndns.org>
Junio C Hamano schrieb:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> For this to work, apply has to be called with --unidiff-zero, since
>> the new hunks can start or stop with a "-" or "+" line.
>
> You do not have to do "unidiff zero". Suppose you have this hunk you
> need to split.
>
> diff --git a/read-cache.c b/read-cache.c
> index 7db5588..4d12073 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -12,8 +12,8 @@
> /* Index extensions.
> *
> * The first letter should be 'A'..'Z' for extensions that are not
> - * necessary for a correct operation (i.e. optimization data).
> - * When new extensions are added that _needs_ to be understood in
> + * necessary for a correct operation (that is, optimization data).
> + * When new extensions are added that needs to be understood in
> * order to correctly interpret the index file, pick character that
> * is outside the range, to cause the reader to abort.
> */
...
> That is, , if you want to do finer grained hunk splitting than what "git
> add -p" lets you do, you do _not_ let user specify "I want to split the
> hunk into two, before this point and after this point". Instead, let
> the user pick zero or more '-' line and zero or more '+' line, and
> adjust the context around it. An unpicked '-' line becomes the common
> context, and an unpicked '+' line disappears. After that, you recount
> the diff. That way, you do not have to do any "unidiff zero" cop-out.
In this case I would expect two adjacent hunks: one that covers the selected
changes, another one with the remaining changes, but each against the original:
@@ -12,7 +12,7 @@
/* Index extensions.
*
* The first letter should be 'A'..'Z' for extensions that are not
- * necessary for a correct operation (i.e. optimization data).
+ * necessary for a correct operation (that is, optimization data).
* When new extensions are added that _needs_ to be understood in
* order to correctly interpret the index file, pick character that
* is outside the range, to cause the reader to abort.
@@ -13,7 +13,7 @@
*
* The first letter should be 'A'..'Z' for extensions that are not
* necessary for a correct operation (i.e. optimization data).
- * When new extensions are added that _needs_ to be understood in
+ * When new extensions are added that needs to be understood in
* order to correctly interpret the index file, pick character that
* is outside the range, to cause the reader to abort.
*/
Then I can stage either one. After that operation, git-gui refreshes the
patch display. This is now the time where the hunk that was not staged
should be updated to reflect the correct diff against the staged hunk.
-- Hannes
^ permalink raw reply
* [PATCH 1/2] parseopt: Enforce the use of the sticked form for optional arguments.
From: Pierre Habouzit @ 2007-12-13 10:26 UTC (permalink / raw)
To: Jeff King, git, Junio C Hamano
In-Reply-To: <20071213093536.GC12398@artemis.madism.org>
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
parse-options.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index e12b428..7a08a0c 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -89,7 +89,7 @@ static int get_value(struct optparse_t *p,
*(const char **)opt->value = NULL;
return 0;
}
- if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-')) {
+ if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
*(const char **)opt->value = (const char *)opt->defval;
return 0;
}
@@ -103,7 +103,7 @@ static int get_value(struct optparse_t *p,
return (*opt->callback)(opt, NULL, 1);
if (opt->flags & PARSE_OPT_NOARG)
return (*opt->callback)(opt, NULL, 0);
- if (opt->flags & PARSE_OPT_OPTARG && (!arg || *arg == '-'))
+ if (opt->flags & PARSE_OPT_OPTARG && !p->opt)
return (*opt->callback)(opt, NULL, 0);
if (!arg)
return opterror(opt, "requires a value", flags);
@@ -114,7 +114,7 @@ static int get_value(struct optparse_t *p,
*(int *)opt->value = 0;
return 0;
}
- if (opt->flags & PARSE_OPT_OPTARG && (!arg || !isdigit(*arg))) {
+ if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
*(int *)opt->value = opt->defval;
return 0;
}
--
1.5.3.7.2249.g89c99-dirty
^ permalink raw reply related
* [PATCH 2/2] parseopt: Add a gitcli(5) man page.
From: Pierre Habouzit @ 2007-12-13 10:27 UTC (permalink / raw)
To: Jeff King, git, Junio C Hamano
In-Reply-To: <20071213102636.GD12398@artemis.madism.org>
This page should hold every information about the git ways to parse command
lines, and best practices to be used for scripting.
Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
Documentation/Makefile | 2 +-
Documentation/gitcli.txt | 104 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 105 insertions(+), 1 deletions(-)
create mode 100644 Documentation/gitcli.txt
diff --git a/Documentation/Makefile b/Documentation/Makefile
index 37ec355..f58f947 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -2,7 +2,7 @@ MAN1_TXT= \
$(filter-out $(addsuffix .txt, $(ARTICLES) $(SP_ARTICLES)), \
$(wildcard git-*.txt)) \
gitk.txt
-MAN5_TXT=gitattributes.txt gitignore.txt gitmodules.txt
+MAN5_TXT=gitattributes.txt gitignore.txt gitcli.txt gitmodules.txt
MAN7_TXT=git.txt
MAN_TXT = $(MAN1_TXT) $(MAN5_TXT) $(MAN7_TXT)
diff --git a/Documentation/gitcli.txt b/Documentation/gitcli.txt
new file mode 100644
index 0000000..3b3f5e4
--- /dev/null
+++ b/Documentation/gitcli.txt
@@ -0,0 +1,104 @@
+gitcli(5)
+=========
+
+NAME
+----
+gitcli - git command line interface and its usual conventions
+
+SYNOPSIS
+--------
+gitcli
+
+
+DESCRIPTION
+-----------
+This manual intends to describe best practice in how to use git CLI. Here are
+the rules that you should follow when you are scripting git:
+
+ * it's preferred to use the non dashed form of git commands, which means that
+ you should prefer `"git foo"` to `"git-foo"`.
+
+ * splitting short option switches in separate atoms (prefer `"git foo -a -b"`
+ to `"git foo -ab"`, the latter may not even work).
+
+ * when a command line switch takes an argument, use the 'sticked' form, which
+ means that you must prefer `"git foo -oArg"` to `"git foo -o Arg"` for short
+ option switches, and `"git foo --long-opt=Arg"` to `"git foo --long-opt Arg"`
+ for long switches.
+
+
+ENHANCED CLI
+------------
+From the git 1.5.4 series and further, git commands (not all of them at the
+time of the writing though) come with an enhanced option parser with nice
+facilities. Here is an exhaustive list of them
+
+Magic Options
+~~~~~~~~~~~~~
+Commands which have the enhanced option parser activated all understand a
+couple of magic command line switches:
+
+-h::
+ gives a pretty printed usage of the command.
++
+---------------------------------------------
+$ git describe -h
+usage: git-describe [options] <committish>*
+
+ --contains find the tag that comes after the commit
+ --debug debug search strategy on stderr
+ --all use any ref in .git/refs
+ --tags use any tag in .git/refs/tags
+ --abbrev [<n>] use <n> digits to display SHA-1s
+ --candidates <n> consider <n> most recent tags (default: 10)
+---------------------------------------------
+
+--help-all::
+ Some git commands takes options that are only used for plumbing or that
+ are deprecated, and such options are hidden from the default usage. This
+ switch gives the full list of options.
+
+
+Negating options
+~~~~~~~~~~~~~~~~
+Another things to keep in mind is that long options can be negated. For
+example, `"git branch"` has the option `"--track"` which is 'on' by default. You
+can use `"--no-track"` to override that behaviour. The same goes for `"--color"`
+and `"--no-color"`.
+
+
+Aggregating short options
+~~~~~~~~~~~~~~~~~~~~~~~~~
+Commands that support the enhanced option parser allow you to aggregate short
+options. This means that you can for example use `"git rm -rf"` or
+`"git clean -fdx"`.
+
+
+Separating argument from the switch
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+Also for option switches that take a mandatory argument, you can separate it
+from the switch. That means that all the following uses are correct:
+
+----------------------------
+$ git foo --long-opt=Arg
+$ git foo --long-opt Arg
+$ git foo -oArg
+$ git foo -o Arg
+----------------------------
+
+However, this is *NOT* possible for switches with an optionnal value, where the
+'sticked' form must be used:
+----------------------------
+$ git describe --abbrev HEAD # correct
+$ git describe --abbrev=10 HEAD # correct
+$ git describe --abbrev 10 HEAD # NOT WHAT YOU MEANT
+----------------------------
+
+
+Documentation
+-------------
+Documentation by Pierre Habouzit.
+
+GIT
+---
+Part of the gitlink:git[7] suite
--
1.5.3.7.2249.g89c99-dirty
^ permalink raw reply related
* Re: [PATCH 2/2] parseopt: Add a gitcli(5) man page.
From: Wincent Colaiuta @ 2007-12-13 11:04 UTC (permalink / raw)
To: Pierre Habouzit; +Cc: Jeff King, git, Junio C Hamano
In-Reply-To: <20071213102724.GE12398@artemis.madism.org>
El 13/12/2007, a las 11:27, Pierre Habouzit escribió:
> This page should hold every information about the git ways to parse
> command
> lines, and best practices to be used for scripting.
Some feedback from a native English speaker follows...
> @@ -0,0 +1,104 @@
> +gitcli(5)
> +=========
> +
> +NAME
> +----
> +gitcli - git command line interface and its usual conventions
"git command line interface and conventions" sounds better; the
"usual" is redundant.
Or did you mean "*usage* conventions"? If that is the case, "git
command line interface and usage" is better, but just "git command
line interface" is enough.
> +DESCRIPTION
> +-----------
> +This manual intends to describe best practice in how to use git
> CLI. Here are
> +the rules that you should follow when you are scripting git:
Suggest "how to use the git CLI".
> + * it's preferred to use the non dashed form of git commands, which
> means that
> + you should prefer `"git foo"` to `"git-foo"`.
"non-dashed", and in any case, this could be more concise. How about:
* the non-dashed form of git commands is preferred; use `"git foo"`
rather than
`"git-foo"`
> + * splitting short option switches in separate atoms (prefer `"git
> foo -a -b"`
> + to `"git foo -ab"`, the latter may not even work).
"*split* short option switches *into* separate atoms"
And the comma before "the latter may not even work" should be a semi-
colon.
> + * when a command line switch takes an argument, use the 'sticked'
> form, which
> + means that you must prefer `"git foo -oArg"` to `"git foo -o
> Arg"` for short
> + option switches, and `"git foo --long-opt=Arg"` to `"git foo --
> long-opt Arg"`
> + for long switches.
Again this could be more concise. Instead of:
"which means that you must prefer .... to ..."
you could just say:
"use ... instead of ..."
> +ENHANCED CLI
> +------------
> +From the git 1.5.4 series and further, git commands (not all of
> them at the
> +time of the writing though) come with an enhanced option parser
> with nice
> +facilities. Here is an exhaustive list of them
How about:
"From git 1.5.4 onwards, many git commands come with an enhanced
option parser..."
> +Magic Options
> +~~~~~~~~~~~~~
> +Commands which have the enhanced option parser activated all
> understand a
> +couple of magic command line switches:
"Commands which use the enhanced option parser all understand..."
> +
> +-h::
> + gives a pretty printed usage of the command.
"pretty-printed"
> +--help-all::
> + Some git commands takes options that are only used for plumbing or
> that
> + are deprecated, and such options are hidden from the default
> usage. This
> + switch gives the full list of options.
"Some git commands *take* options that are deprecated or used only
*by* plumbing"
And:
"such options are *not included in* the default usage" ("hidden from"
sounds awkward).
> +Negating options
> +~~~~~~~~~~~~~~~~
> +Another things to keep in mind is that long options can be negated.
"Another *thing*"
But you could replace the whole sentence with just:
"Long options can be negated."
> +Separating argument from the switch
> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
"Separating *the* argument from the switch"
Or if you prefer
"Separating *arguments* from the *switches*"
> +However, this is *NOT* possible for switches with an optionnal
> value, where the
> +'sticked' form must be used:
Typo: "optional"
>
Cheers,
Wincent
^ permalink raw reply
* Re: [PATCH] Teach git-gui to split hunks
From: Johannes Schindelin @ 2007-12-13 12:25 UTC (permalink / raw)
To: Johannes Sixt
Cc: Jason Sewall, Shawn O. Pearce, David, Marco Costalba,
Andy Parkins, git
In-Reply-To: <4760E0CF.1030805@viscovery.net>
Hi,
On Thu, 13 Dec 2007, Johannes Sixt wrote:
> Johannes Schindelin schrieb:
> > When you select the context menu item "Split Hunk" in the diff area,
> > git-gui will now split the current hunk so that a new hunk starts at
> > the current position.
> >
> > For this to work, apply has to be called with --unidiff-zero, since
> > the new hunks can start or stop with a "-" or "+" line.
>
> NACK! --unidiff-zero eats your data.
Did I not make crystal clear that I intended this patch to be cleaned up
first, to not need unidiff-zero?
If so, my sincerest apologies.
THIS PATCH IS NOT MEANT FOR APPLICATION, BUT FOR JASON TO PLAY WITH.
Ciao,
Dscho
^ permalink raw reply
* Re: [PATCH] Teach git-gui to split hunks
From: Johannes Schindelin @ 2007-12-13 12:49 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jason Sewall, Shawn O. Pearce, David, Marco Costalba,
Andy Parkins, git
In-Reply-To: <7vhcin3rv4.fsf@gitster.siamese.dyndns.org>
Hi,
On Thu, 13 Dec 2007, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > For this to work, apply has to be called with --unidiff-zero, since
> > the new hunks can start or stop with a "-" or "+" line.
>
> You do not have to do "unidiff zero". Suppose you have this hunk you
> need to split.
>
> [describes to pick zero or more '-' lines and zero or more '+' lines]
I thought about that, but the UI is not trivial. The UI for my solution
is.
Ciao,
Dscho
^ permalink raw reply
* Re: Something is broken in repack
From: Nguyen Thai Ngoc Duy @ 2007-12-13 13:32 UTC (permalink / raw)
To: Nicolas Pitre; +Cc: Jon Smirl, Junio C Hamano, gcc, Git Mailing List
In-Reply-To: <alpine.LFD.0.99999.0712120743040.555@xanadu.home>
On Dec 12, 2007 10:48 PM, Nicolas Pitre <nico@cam.org> wrote:
> In the mean time you might have to use only one thread and lots of
> memory to repack the gcc repo, or find the perfect memory allocator to
> be used with Git. After all, packing the whole gcc history to around
> 230MB is quite a stunt but it requires sufficient resources to
> achieve it. Fortunately, like Linus said, such a wholesale repack is not
> something that most users have to do anyway.
Is there an alternative to "git repack -a -d" that repacks everything
but the first pack?
--
Duy
^ permalink raw reply
* [PATCH 0/5] "diff --check" and whitespace enhancements
From: Wincent Colaiuta @ 2007-12-13 13:32 UTC (permalink / raw)
To: git; +Cc: gitster
This is a repost of the topic I posted yesterday, rebased on top
of master and omitting the changes already applied to it.
Of the series the most interesting is [3/5], where I implement
Junio's suggestion of further refactoring to get rid of
emit_line_with_ws().
[1/5] "diff --check" should affect exit status
[2/5] New version of pre-commit hook
[3/5] Unify whitespace checking
[4/5] Make "diff --check" output match "git apply"
[5/5] Add tests for "git diff --check" with core.whitespace options
I think there are still some opportunities for further refactoring
and clean-up, but this is a good start.
Overall the refactoring saves duplication and reduces the line
count (apart from the tests, of course, where line count goes
unequivocally up).
Documentation/diff-options.txt | 4 +-
builtin-apply.c | 56 +++----------
builtin-diff-files.c | 2 +
builtin-diff-index.c | 2 +
builtin-diff-tree.c | 28 +++---
builtin-diff.c | 3 +-
cache.h | 4 +
diff.c | 172 +++++++++---------------------------
diff.h | 1 +
t/t4015-diff-whitespace.sh | 189 +++++++++++++++++++++++++++++++++++++++-
templates/hooks--pre-commit | 67 ++-------------
ws.c | 105 ++++++++++++++++++++++
12 files changed, 379 insertions(+), 254 deletions(-)
Cheers,
Wincent
^ permalink raw reply
* [PATCH 1/5] "diff --check" should affect exit status
From: Wincent Colaiuta @ 2007-12-13 13:32 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197552751-53480-1-git-send-email-win@wincent.com>
"git diff" has a --check option that can be used to check for whitespace
problems but it only reported by printing warnings to the
console.
Now when the --check option is used we give a non-zero exit status,
making "git diff --check" nicer to use in scripts and hooks.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
Documentation/diff-options.txt | 4 +-
builtin-diff-files.c | 2 +
builtin-diff-index.c | 2 +
builtin-diff-tree.c | 28 ++++----
builtin-diff.c | 3 +-
diff.c | 37 +++++++----
diff.h | 1 +
t/t4015-diff-whitespace.sh | 137 +++++++++++++++++++++++++++++++++++++++-
8 files changed, 184 insertions(+), 30 deletions(-)
diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
index 5d22b7b..9ecc1d7 100644
--- a/Documentation/diff-options.txt
+++ b/Documentation/diff-options.txt
@@ -93,7 +93,9 @@ endif::git-format-patch[]
--check::
Warn if changes introduce trailing whitespace
- or an indent that uses a space before a tab.
+ or an indent that uses a space before a tab. Exits with
+ non-zero status if problems are found. Not compatible with
+ --exit-code.
--full-index::
Instead of the first handful characters, show full
diff --git a/builtin-diff-files.c b/builtin-diff-files.c
index 046b7e3..4afc872 100644
--- a/builtin-diff-files.c
+++ b/builtin-diff-files.c
@@ -33,5 +33,7 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
result = run_diff_files_cmd(&rev, argc, argv);
if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+ if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+ return DIFF_OPT_TST(&rev.diffopt, CHECK_FAILED) != 0;
return result;
}
diff --git a/builtin-diff-index.c b/builtin-diff-index.c
index 556c506..532b284 100644
--- a/builtin-diff-index.c
+++ b/builtin-diff-index.c
@@ -46,5 +46,7 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
result = run_diff_index(&rev, cached);
if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
+ if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+ return DIFF_OPT_TST(&rev.diffopt, CHECK_FAILED) != 0;
return result;
}
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index 2e13716..9ab90cb 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -117,23 +117,23 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
break;
}
- if (!read_stdin)
- return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
- && DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
+ if (read_stdin) {
+ if (opt->diffopt.detect_rename)
+ opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
+ DIFF_SETUP_USE_CACHE);
+ while (fgets(line, sizeof(line), stdin)) {
+ unsigned char sha1[20];
- if (opt->diffopt.detect_rename)
- opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
- DIFF_SETUP_USE_CACHE);
- while (fgets(line, sizeof(line), stdin)) {
- unsigned char sha1[20];
-
- if (get_sha1_hex(line, sha1)) {
- fputs(line, stdout);
- fflush(stdout);
+ if (get_sha1_hex(line, sha1)) {
+ fputs(line, stdout);
+ fflush(stdout);
+ }
+ else
+ diff_tree_stdin(line);
}
- else
- diff_tree_stdin(line);
}
+ if (opt->diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+ return DIFF_OPT_TST(&opt->diffopt, CHECK_FAILED) != 0;
return DIFF_OPT_TST(&opt->diffopt, EXIT_WITH_STATUS)
&& DIFF_OPT_TST(&opt->diffopt, HAS_CHANGES);
}
diff --git a/builtin-diff.c b/builtin-diff.c
index 55fb84c..86d01a3 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -353,7 +353,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
ent, ents);
if (DIFF_OPT_TST(&rev.diffopt, EXIT_WITH_STATUS))
result = DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES) != 0;
-
+ if (rev.diffopt.output_format & DIFF_FORMAT_CHECKDIFF)
+ return DIFF_OPT_TST(&rev.diffopt, CHECK_FAILED) != 0;
if (1 < rev.diffopt.skip_stat_unmatch)
refresh_index_quietly();
return result;
diff --git a/diff.c b/diff.c
index 9c79ee2..39109a6 100644
--- a/diff.c
+++ b/diff.c
@@ -1031,6 +1031,7 @@ struct checkdiff_t {
const char *filename;
int lineno, color_diff;
unsigned ws_rule;
+ unsigned status;
};
static void checkdiff_consume(void *priv, char *line, unsigned long len)
@@ -1064,6 +1065,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
white_space_at_end = 1;
if (space_before_tab || white_space_at_end) {
+ data->status = 1;
printf("%s:%d: %s", data->filename, data->lineno, ws);
if (space_before_tab) {
printf("space before tab");
@@ -1454,7 +1456,7 @@ static void builtin_diffstat(const char *name_a, const char *name_b,
diff_free_filespec_data(two);
}
-static void builtin_checkdiff(const char *name_a, const char *name_b,
+static int builtin_checkdiff(const char *name_a, const char *name_b,
struct diff_filespec *one,
struct diff_filespec *two, struct diff_options *o)
{
@@ -1462,7 +1464,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
struct checkdiff_t data;
if (!two)
- return;
+ return 0;
memset(&data, 0, sizeof(data));
data.xm.consume = checkdiff_consume;
@@ -1491,6 +1493,7 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
free_and_return:
diff_free_filespec_data(one);
diff_free_filespec_data(two);
+ return data.status;
}
struct diff_filespec *alloc_filespec(const char *path)
@@ -2075,14 +2078,14 @@ static void run_diffstat(struct diff_filepair *p, struct diff_options *o,
builtin_diffstat(name, other, p->one, p->two, diffstat, o, complete_rewrite);
}
-static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
+static int run_checkdiff(struct diff_filepair *p, struct diff_options *o)
{
const char *name;
const char *other;
if (DIFF_PAIR_UNMERGED(p)) {
/* unmerged */
- return;
+ return 0;
}
name = p->one->path;
@@ -2091,7 +2094,7 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
diff_fill_sha1_info(p->one);
diff_fill_sha1_info(p->two);
- builtin_checkdiff(name, other, p->one, p->two, o);
+ return builtin_checkdiff(name, other, p->one, p->two, o);
}
void diff_setup(struct diff_options *options)
@@ -2121,7 +2124,12 @@ int diff_setup_done(struct diff_options *options)
if (options->output_format & DIFF_FORMAT_NAME_STATUS)
count++;
if (options->output_format & DIFF_FORMAT_CHECKDIFF)
+ {
count++;
+ if (DIFF_OPT_TST(options, QUIET) ||
+ DIFF_OPT_TST(options, EXIT_WITH_STATUS))
+ die("--check may not be used with --quiet or --exit-code");
+ }
if (options->output_format & DIFF_FORMAT_NO_OUTPUT)
count++;
if (count > 1)
@@ -2588,17 +2596,17 @@ static void diff_flush_stat(struct diff_filepair *p, struct diff_options *o,
run_diffstat(p, o, diffstat);
}
-static void diff_flush_checkdiff(struct diff_filepair *p,
+static int diff_flush_checkdiff(struct diff_filepair *p,
struct diff_options *o)
{
if (diff_unmodified_pair(p))
- return;
+ return 0;
if ((DIFF_FILE_VALID(p->one) && S_ISDIR(p->one->mode)) ||
(DIFF_FILE_VALID(p->two) && S_ISDIR(p->two->mode)))
- return; /* no tree diffs in patch format */
+ return 0; /* no tree diffs in patch format */
- run_checkdiff(p, o);
+ return run_checkdiff(p, o);
}
int diff_queue_is_empty(void)
@@ -2717,16 +2725,19 @@ static int check_pair_status(struct diff_filepair *p)
}
}
-static void flush_one_pair(struct diff_filepair *p, struct diff_options *opt)
+static int flush_one_pair(struct diff_filepair *p, struct diff_options *opt)
{
int fmt = opt->output_format;
if (fmt & DIFF_FORMAT_CHECKDIFF)
- diff_flush_checkdiff(p, opt);
+ return diff_flush_checkdiff(p, opt);
else if (fmt & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS))
diff_flush_raw(p, opt);
else if (fmt & DIFF_FORMAT_NAME)
write_name_quoted(p->two->path, stdout, opt->line_termination);
+
+ /* return value only matters with DIFF_FORMAT_CHECKDIFF */
+ return 0;
}
static void show_file_mode_name(const char *newdelete, struct diff_filespec *fs)
@@ -2965,8 +2976,8 @@ void diff_flush(struct diff_options *options)
DIFF_FORMAT_CHECKDIFF)) {
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
- if (check_pair_status(p))
- flush_one_pair(p, options);
+ if (check_pair_status(p) && flush_one_pair(p, options))
+ DIFF_OPT_SET(options, CHECK_FAILED);
}
separator++;
}
diff --git a/diff.h b/diff.h
index a52496a..5d50d93 100644
--- a/diff.h
+++ b/diff.h
@@ -59,6 +59,7 @@ typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
#define DIFF_OPT_ALLOW_EXTERNAL (1 << 13)
#define DIFF_OPT_EXIT_WITH_STATUS (1 << 14)
#define DIFF_OPT_REVERSE_DIFF (1 << 15)
+#define DIFF_OPT_CHECK_FAILED (1 << 16)
#define DIFF_OPT_TST(opts, flag) ((opts)->flags & DIFF_OPT_##flag)
#define DIFF_OPT_SET(opts, flag) ((opts)->flags |= DIFF_OPT_##flag)
#define DIFF_OPT_CLR(opts, flag) ((opts)->flags &= ~DIFF_OPT_##flag)
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index 6adf9d1..dc538b3 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -117,7 +117,6 @@ EOF
git diff -b > out
test_expect_success 'another test, with -b' 'git diff expect out'
-
test_expect_success 'check mixed spaces and tabs in indent' '
# This is indented with SP HT SP.
@@ -126,4 +125,140 @@ test_expect_success 'check mixed spaces and tabs in indent' '
'
+test_expect_success 'check with no whitespace errors' '
+
+ git commit -m "snapshot" &&
+ echo "foo();" > x &&
+ git diff --check
+
+'
+
+test_expect_failure 'check with trailing whitespace' '
+
+ echo "foo(); " > x &&
+ git diff --check
+
+'
+
+test_expect_failure 'check with space before tab in indent' '
+
+ # indent has space followed by hard tab
+ echo " foo();" > x &&
+ git diff --check
+
+'
+
+test_expect_failure '--check and --exit-code are exclusive' '
+
+ git checkout x &&
+ git diff --check --exit-code
+
+'
+
+test_expect_failure '--check and --quiet are exclusive' '
+
+ git diff --check --quiet
+
+'
+
+test_expect_success 'check staged with no whitespace errors' '
+
+ echo "foo();" > x &&
+ git add x &&
+ git diff --cached --check
+
+'
+
+test_expect_failure 'check staged with trailing whitespace' '
+
+ echo "foo(); " > x &&
+ git add x &&
+ git diff --cached --check
+
+'
+
+test_expect_failure 'check staged with space before tab in indent' '
+
+ # indent has space followed by hard tab
+ echo " foo();" > x &&
+ git add x &&
+ git diff --cached --check
+
+'
+
+test_expect_success 'check with no whitespace errors (diff-index)' '
+
+ echo "foo();" > x &&
+ git add x &&
+ git diff-index --check HEAD
+
+'
+
+test_expect_failure 'check with trailing whitespace (diff-index)' '
+
+ echo "foo(); " > x &&
+ git add x &&
+ git diff-index --check HEAD
+
+'
+
+test_expect_failure 'check with space before tab in indent (diff-index)' '
+
+ # indent has space followed by hard tab
+ echo " foo();" > x &&
+ git add x &&
+ git diff-index --check HEAD
+
+'
+
+test_expect_success 'check staged with no whitespace errors (diff-index)' '
+
+ echo "foo();" > x &&
+ git add x &&
+ git diff-index --cached --check HEAD
+
+'
+
+test_expect_failure 'check staged with trailing whitespace (diff-index)' '
+
+ echo "foo(); " > x &&
+ git add x &&
+ git diff-index --cached --check HEAD
+
+'
+
+test_expect_failure 'check staged with space before tab in indent (diff-index)' '
+
+ # indent has space followed by hard tab
+ echo " foo();" > x &&
+ git add x &&
+ git diff-index --cached --check HEAD
+
+'
+
+test_expect_success 'check with no whitespace errors (diff-tree)' '
+
+ echo "foo();" > x &&
+ git commit -m "new commit" x &&
+ git diff-tree --check HEAD^ HEAD
+
+'
+
+test_expect_failure 'check with trailing whitespace (diff-tree)' '
+
+ echo "foo(); " > x &&
+ git commit -m "another commit" x &&
+ git diff-tree --check HEAD^ HEAD
+
+'
+
+test_expect_failure 'check with space before tab in indent (diff-tree)' '
+
+ # indent has space followed by hard tab
+ echo " foo();" > x &&
+ git commit -m "yet another" x &&
+ git diff-tree --check HEAD^ HEAD
+
+'
+
test_done
--
1.5.4.rc0.4.g50348
^ permalink raw reply related
* [PATCH 5/5] Add tests for "git diff --check" with core.whitespace options
From: Wincent Colaiuta @ 2007-12-13 13:32 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197552751-53480-5-git-send-email-win@wincent.com>
Make sure that "git diff --check" does the right thing when the
core.whitespace options are set.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
t/t4015-diff-whitespace.sh | 50 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 50 insertions(+), 0 deletions(-)
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index a0a47dd..cb90ee7 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -261,4 +261,54 @@ test_expect_failure 'check with space before tab in indent (diff-tree)' '
'
+test_expect_success 'check trailing whitespace (trailing-space: off)' '
+
+ git config core.whitespace "-trailing-space" &&
+ echo "foo (); " > x &&
+ git diff --check
+
+'
+
+test_expect_failure 'check trailing whitespace (trailing-space: on)' '
+
+ git config core.whitespace "trailing-space" &&
+ echo "foo (); " > x &&
+ git diff --check
+
+'
+
+test_expect_success 'check space before tab in indent (space-before-tab: off)' '
+
+ # indent contains space followed by HT
+ git config core.whitespace "-space-before-tab" &&
+ echo " foo ();" > x &&
+ git diff --check
+
+'
+
+test_expect_failure 'check space before tab in indent (space-before-tab: on)' '
+
+ # indent contains space followed by HT
+ git config core.whitespace "space-before-tab" &&
+ echo " foo (); " > x &&
+ git diff --check
+
+'
+
+test_expect_success 'check spaces as indentation (indent-with-non-tab: off)' '
+
+ git config core.whitespace "-indent-with-non-tab"
+ echo " foo ();" > x &&
+ git diff --check
+
+'
+
+test_expect_failure 'check spaces as indentation (indent-with-non-tab: on)' '
+
+ git config core.whitespace "indent-with-non-tab" &&
+ echo " foo ();" > x &&
+ git diff --check
+
+'
+
test_done
--
1.5.4.rc0.4.g50348
^ permalink raw reply related
* [PATCH 3/5] Unify whitespace checking
From: Wincent Colaiuta @ 2007-12-13 13:32 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197552751-53480-3-git-send-email-win@wincent.com>
This commit unifies three separate places where whitespace checking was
performed:
- the whitespace checking previously done in builtin-apply.c is
extracted into a function in ws.c
- the equivalent logic in "git diff" is removed
- the emit_line_with_ws() function is also removed because that also
rechecks the shitespace, and its functionality is rolled into ws.c
The new function is called check_and_emit_line() and it does two things:
checks a line for whitespace errors and optionally emits it. The checking
is based on lines of content rather than patch lines (in other words, the
caller must strip the leading "+" or "-"); this was suggested by Junio on
the mailing list to allow for a future extension to "git show" to display
whitespace errors in blobs.
At the same time we teach it to report all classes of whitespace errors
found for a given line rather than reporting only the first found error.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
builtin-apply.c | 54 +++--------------
cache.h | 4 +
diff.c | 138 ++++++--------------------------------------
t/t4015-diff-whitespace.sh | 2 +-
ws.c | 105 +++++++++++++++++++++++++++++++++
5 files changed, 139 insertions(+), 164 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index f2e9a33..ab393f3 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -900,56 +900,22 @@ static int find_header(char *line, unsigned long size, int *hdrsize, struct patc
static void check_whitespace(const char *line, int len, unsigned ws_rule)
{
- const char *err = "Adds trailing whitespace";
- int seen_space = 0;
- int i;
-
- /*
- * We know len is at least two, since we have a '+' and we
- * checked that the last character was a '\n' before calling
- * this function. That is, an addition of an empty line would
- * check the '+' here. Sneaky...
- */
- if ((ws_rule & WS_TRAILING_SPACE) && isspace(line[len-2]))
- goto error;
-
- /*
- * Make sure that there is no space followed by a tab in
- * indentation.
- */
- if (ws_rule & WS_SPACE_BEFORE_TAB) {
- err = "Space in indent is followed by a tab";
- for (i = 1; i < len; i++) {
- if (line[i] == '\t') {
- if (seen_space)
- goto error;
- }
- else if (line[i] == ' ')
- seen_space = 1;
- else
- break;
- }
- }
-
- /*
- * Make sure that the indentation does not contain more than
- * 8 spaces.
- */
- if ((ws_rule & WS_INDENT_WITH_NON_TAB) &&
- (8 < len) && !strncmp("+ ", line, 9)) {
- err = "Indent more than 8 places with spaces";
- goto error;
- }
- return;
+ char *err;
+ unsigned result = check_and_emit_line(line + 1, len - 1, ws_rule,
+ NULL, NULL, NULL, NULL);
+ if (!result)
+ return;
- error:
whitespace_error++;
if (squelch_whitespace_errors &&
squelch_whitespace_errors < whitespace_error)
;
- else
+ else {
+ err = whitespace_error_string(result);
fprintf(stderr, "%s.\n%s:%d:%.*s\n",
- err, patch_input_file, linenr, len-2, line+1);
+ err, patch_input_file, linenr, len - 2, line + 1);
+ free(err);
+ }
}
/*
diff --git a/cache.h b/cache.h
index 27d90fe..39331c2 100644
--- a/cache.h
+++ b/cache.h
@@ -655,6 +655,10 @@ void shift_tree(const unsigned char *, const unsigned char *, unsigned char *, i
extern unsigned whitespace_rule_cfg;
extern unsigned whitespace_rule(const char *);
extern unsigned parse_whitespace_rule(const char *);
+extern unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
+ FILE *stream, const char *set,
+ const char *reset, const char *ws);
+extern char *whitespace_error_string(unsigned ws);
/* ls-files */
int pathspec_match(const char **spec, char *matched, const char *filename, int skiplen);
diff --git a/diff.c b/diff.c
index 39109a6..d96a4ac 100644
--- a/diff.c
+++ b/diff.c
@@ -486,88 +486,9 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix)
static void emit_line(const char *set, const char *reset, const char *line, int len)
{
- if (len > 0 && line[len-1] == '\n')
- len--;
fputs(set, stdout);
fwrite(line, len, 1, stdout);
- puts(reset);
-}
-
-static void emit_line_with_ws(int nparents,
- const char *set, const char *reset, const char *ws,
- const char *line, int len, unsigned ws_rule)
-{
- int col0 = nparents;
- int last_tab_in_indent = -1;
- int last_space_in_indent = -1;
- int i;
- int tail = len;
- int need_highlight_leading_space = 0;
- /*
- * The line is a newly added line. Does it have funny leading
- * whitespaces? In indent, SP should never precede a TAB. In
- * addition, under "indent with non tab" rule, there should not
- * be more than 8 consecutive spaces.
- */
- for (i = col0; i < len; i++) {
- if (line[i] == '\t') {
- last_tab_in_indent = i;
- if ((ws_rule & WS_SPACE_BEFORE_TAB) &&
- 0 <= last_space_in_indent)
- need_highlight_leading_space = 1;
- }
- else if (line[i] == ' ')
- last_space_in_indent = i;
- else
- break;
- }
- if ((ws_rule & WS_INDENT_WITH_NON_TAB) &&
- 0 <= last_space_in_indent &&
- last_tab_in_indent < 0 &&
- 8 <= (i - col0)) {
- last_tab_in_indent = i;
- need_highlight_leading_space = 1;
- }
- fputs(set, stdout);
- fwrite(line, col0, 1, stdout);
fputs(reset, stdout);
- if (((i == len) || line[i] == '\n') && i != col0) {
- /* The whole line was indent */
- emit_line(ws, reset, line + col0, len - col0);
- return;
- }
- i = col0;
- if (need_highlight_leading_space) {
- while (i < last_tab_in_indent) {
- if (line[i] == ' ') {
- fputs(ws, stdout);
- putchar(' ');
- fputs(reset, stdout);
- }
- else
- putchar(line[i]);
- i++;
- }
- }
- tail = len - 1;
- if (line[tail] == '\n' && i < tail)
- tail--;
- if (ws_rule & WS_TRAILING_SPACE) {
- while (i < tail) {
- if (!isspace(line[tail]))
- break;
- tail--;
- }
- }
- if ((i < tail && line[tail + 1] != '\n')) {
- /* This has whitespace between tail+1..len */
- fputs(set, stdout);
- fwrite(line + i, tail - i + 1, 1, stdout);
- fputs(reset, stdout);
- emit_line(ws, reset, line + tail + 1, len - tail - 1);
- }
- else
- emit_line(set, reset, line + i, len - i);
}
static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
@@ -577,9 +498,13 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons
if (!*ws)
emit_line(set, reset, line, len);
- else
- emit_line_with_ws(ecbdata->nparents, set, reset, ws,
- line, len, ecbdata->ws_rule);
+ else {
+ /* Emit just the prefix, then the rest. */
+ emit_line(set, reset, line, ecbdata->nparents);
+ (void)check_and_emit_line(line + ecbdata->nparents,
+ len - ecbdata->nparents, ecbdata->ws_rule,
+ stdout, set, reset, ws);
+ }
}
static void fn_out_consume(void *priv, char *line, unsigned long len)
@@ -1040,45 +965,20 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
const char *ws = diff_get_color(data->color_diff, DIFF_WHITESPACE);
const char *reset = diff_get_color(data->color_diff, DIFF_RESET);
const char *set = diff_get_color(data->color_diff, DIFF_FILE_NEW);
+ char *err;
if (line[0] == '+') {
- int i, spaces = 0, space_before_tab = 0, white_space_at_end = 0;
-
- /* check space before tab */
- for (i = 1; i < len; i++) {
- if (line[i] == ' ')
- spaces++;
- else if (line[i] == '\t') {
- if (spaces) {
- space_before_tab = 1;
- break;
- }
- }
- else
- break;
- }
-
- /* check whitespace at line end */
- if (line[len - 1] == '\n')
- len--;
- if (isspace(line[len - 1]))
- white_space_at_end = 1;
-
- if (space_before_tab || white_space_at_end) {
- data->status = 1;
- printf("%s:%d: %s", data->filename, data->lineno, ws);
- if (space_before_tab) {
- printf("space before tab");
- if (white_space_at_end)
- putchar(',');
- }
- if (white_space_at_end)
- printf("whitespace at end");
- printf(":%s ", reset);
- emit_line_with_ws(1, set, reset, ws, line, len,
- data->ws_rule);
- }
-
+ data->status = check_and_emit_line(line + 1, len - 1,
+ data->ws_rule, NULL, NULL, NULL, NULL);
+ if (!data->status)
+ return;
+ err = whitespace_error_string(data->status);
+ printf("%s:%d: %s%s:%s ", data->filename, data->lineno,
+ ws, err, reset);
+ free(err);
+ emit_line(set, reset, line, 1);
+ (void)check_and_emit_line(line + 1, len - 1, data->ws_rule,
+ stdout, set, reset, ws);
data->lineno++;
} else if (line[0] == ' ')
data->lineno++;
diff --git a/t/t4015-diff-whitespace.sh b/t/t4015-diff-whitespace.sh
index dc538b3..a0a47dd 100755
--- a/t/t4015-diff-whitespace.sh
+++ b/t/t4015-diff-whitespace.sh
@@ -121,7 +121,7 @@ test_expect_success 'check mixed spaces and tabs in indent' '
# This is indented with SP HT SP.
echo " foo();" > x &&
- git diff --check | grep "space before tab"
+ git diff --check | grep "Space in indent is followed by a tab"
'
diff --git a/ws.c b/ws.c
index 52c10ca..a0ce860 100644
--- a/ws.c
+++ b/ws.c
@@ -94,3 +94,108 @@ unsigned whitespace_rule(const char *pathname)
return whitespace_rule_cfg;
}
}
+
+/* The returned string should be freed by the caller. */
+char *whitespace_error_string(unsigned ws)
+{
+ struct strbuf err;
+ strbuf_init(&err, 0);
+ if (ws & WS_TRAILING_SPACE)
+ strbuf_addstr(&err, "Adds trailing whitespace");
+ if (ws & WS_SPACE_BEFORE_TAB) {
+ if (err.len)
+ strbuf_addstr(&err, ", ");
+ strbuf_addstr(&err, "Space in indent is followed by a tab");
+ }
+ if (ws & WS_INDENT_WITH_NON_TAB) {
+ if (err.len)
+ strbuf_addstr(&err, ", ");
+ strbuf_addstr(&err, "Indent more than 8 places with spaces");
+ }
+ return strbuf_detach(&err, NULL);
+}
+
+/* If stream is non-NULL, emits the line after checking. */
+unsigned check_and_emit_line(const char *line, int len, unsigned ws_rule,
+ FILE *stream, const char *set,
+ const char *reset, const char *ws)
+{
+ unsigned result = 0;
+ int leading_space = -1;
+ int trailing_whitespace = -1;
+ int trailing_newline = 0;
+ int i;
+
+ /* Logic is simpler if we temporarily ignore the trailing newline. */
+ if (len > 0 && line[len - 1] == '\n') {
+ trailing_newline = 1;
+ len--;
+ }
+
+ /* Check for trailing whitespace. */
+ if (ws_rule & WS_TRAILING_SPACE) {
+ for (i = len - 1; i >= 0; i--) {
+ if (isspace(line[i])) {
+ trailing_whitespace = i;
+ result |= WS_TRAILING_SPACE;
+ }
+ else
+ break;
+ }
+ }
+
+ /* Check for space before tab in initial indent. */
+ for (i = 0; i < len; i++) {
+ if (line[i] == '\t') {
+ if ((ws_rule & WS_SPACE_BEFORE_TAB) &&
+ (leading_space != -1))
+ result |= WS_SPACE_BEFORE_TAB;
+ break;
+ }
+ else if (line[i] == ' ')
+ leading_space = i;
+ else
+ break;
+ }
+
+ /* Check for indent using non-tab. */
+ if ((ws_rule & WS_INDENT_WITH_NON_TAB) && leading_space >= 8)
+ result |= WS_INDENT_WITH_NON_TAB;
+
+ if (stream) {
+ /* Highlight errors in leading whitespace. */
+ if ((result & WS_SPACE_BEFORE_TAB) ||
+ (result & WS_INDENT_WITH_NON_TAB)) {
+ fputs(ws, stream);
+ fwrite(line, leading_space + 1, 1, stream);
+ fputs(reset, stream);
+ leading_space++;
+ }
+ else
+ leading_space = 0;
+
+ /* Now the rest of the line starts at leading_space.
+ * The non-highlighted part ends at trailing_whitespace. */
+ if (trailing_whitespace == -1)
+ trailing_whitespace = len;
+
+ /* Emit non-highlighted (middle) segment. */
+ if (trailing_whitespace - leading_space > 0) {
+ fputs(set, stream);
+ fwrite(line + leading_space,
+ trailing_whitespace - leading_space, 1, stream);
+ fputs(reset, stream);
+ }
+
+ /* Highlight errors in trailing whitespace. */
+ if (trailing_whitespace != len) {
+ fputs(ws, stream);
+ fwrite(line + trailing_whitespace,
+ len - trailing_whitespace, 1, stream);
+ fputs(reset, stream);
+ }
+ if (trailing_newline)
+ fputc('\n', stream);
+ }
+ return result;
+}
--
1.5.4.rc0.4.g50348
^ permalink raw reply related
* [PATCH 4/5] Make "diff --check" output match "git apply"
From: Wincent Colaiuta @ 2007-12-13 13:32 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197552751-53480-4-git-send-email-win@wincent.com>
For consistency, make the two tools report whitespace errors in the
same way (the output of "diff --check" has been tweaked to match
that of "git apply").
Note that although the textual content is basically the same only
"git diff --check" provides a colorized version of the problematic
lines; making "git apply" do colorization will require more extensive
changes (figuring out the diff colorization preferences of the user)
and so that will be a subject for another commit.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
builtin-apply.c | 4 ++--
diff.c | 3 +--
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/builtin-apply.c b/builtin-apply.c
index ab393f3..2edd83b 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -912,8 +912,8 @@ static void check_whitespace(const char *line, int len, unsigned ws_rule)
;
else {
err = whitespace_error_string(result);
- fprintf(stderr, "%s.\n%s:%d:%.*s\n",
- err, patch_input_file, linenr, len - 2, line + 1);
+ fprintf(stderr, "%s:%d: %s.\n%.*s\n",
+ patch_input_file, linenr, err, len - 2, line + 1);
free(err);
}
}
diff --git a/diff.c b/diff.c
index d96a4ac..73a723d 100644
--- a/diff.c
+++ b/diff.c
@@ -973,8 +973,7 @@ static void checkdiff_consume(void *priv, char *line, unsigned long len)
if (!data->status)
return;
err = whitespace_error_string(data->status);
- printf("%s:%d: %s%s:%s ", data->filename, data->lineno,
- ws, err, reset);
+ printf("%s:%d: %s.\n", data->filename, data->lineno, err);
free(err);
emit_line(set, reset, line, 1);
(void)check_and_emit_line(line + 1, len - 1, data->ws_rule,
--
1.5.4.rc0.4.g50348
^ permalink raw reply related
* [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call
From: David Kågedal @ 2007-12-13 13:38 UTC (permalink / raw)
To: catalin.marinas; +Cc: git
This isn't a real patch yet, but it is good enough for my usage.
I have a fair amount of branches, and I noticed that "stg branch -l"
takes ridiculously long to finish. The problem is that it creates
stack objects for all branches, and indiviudally extract information
about them one after one. With 20 branches, it took almost 2 seconds
to run. Compare that with the 0.01 seconds it takes to run "git
branch".
I made a patch that uses "git config --get-regexp" to get the
description and stgit.stackformatversion options for all branches at
once, and ignore the "protected" flag that I don't use. With this
change, I'm almost down to half a second, which almost makes it
usable.
There are still a bunch of redundant invokations of git, but python
startup times are hard to get around.
Maybe someone can help me find a quicker replacement for the
get_protected call?
---
stgit/commands/branch.py | 39 ++++++++++++++++++++++++++++++---------
1 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/stgit/commands/branch.py b/stgit/commands/branch.py
index 50684bb..f4b0c33 100644
--- a/stgit/commands/branch.py
+++ b/stgit/commands/branch.py
@@ -72,21 +72,21 @@ options = [make_option('-c', '--create',
def __is_current_branch(branch_name):
return crt_series.get_name() == branch_name
-def __print_branch(branch_name, length):
+def __print_branch(branch_name, description, version, length):
initialized = ' '
current = ' '
protected = ' '
- branch = stack.Series(branch_name)
+ #branch = stack.Series(branch_name)
- if branch.is_initialised():
+ if version != None:
initialized = 's'
if __is_current_branch(branch_name):
current = '>'
- if branch.get_protected():
- protected = 'p'
+ #if branch.get_protected():
+ # protected = 'p'
out.stdout(current + ' ' + initialized + protected + '\t'
- + branch_name.ljust(length) + ' | ' + branch.get_description())
+ + branch_name.ljust(length) + ' | ' + (description or ''))
def __delete_branch(doomed_name, force = False):
doomed = stack.Series(doomed_name)
@@ -100,6 +100,23 @@ def __delete_branch(doomed_name, force = False):
doomed.delete(force)
out.done()
+class FormatException(StgException):
+ pass
+
+def __get_all_branch_config(key):
+ key = re.escape(key)
+ lines = git.GRun('config', '--get-regexp',
+ r'branch\..*\.'+key).returns([0,1]).output_lines()
+ val_re = re.compile(r'branch\.(.*)\.%s (.*)' % key)
+ result = {}
+ for line in lines:
+ m = val_re.match(line)
+ if not m:
+ raise FormatException("unknown output from git config")
+ branch, data = m.groups()
+ result[branch] = data
+ return result
+
def func(parser, options, args):
if options.create:
@@ -198,11 +215,15 @@ def func(parser, options, args):
branches = git.get_heads()
branches.sort()
+ descriptions = __get_all_branch_config('description')
+ versions = __get_all_branch_config('stgit.stackformatversion')
+
if branches:
out.info('Available branches:')
- max_len = max([len(i) for i in branches])
- for i in branches:
- __print_branch(i, max_len)
+ max_len = max(len(i) for i in branches)
+ for branch in branches:
+ __print_branch(branch, descriptions.get(branch),
+ versions.get(branch), max_len)
else:
out.info('No branches')
return
^ permalink raw reply related
* Re: [PATCH 3/5] Unify whitespace checking
From: Andreas Ericsson @ 2007-12-13 13:40 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: git, gitster
In-Reply-To: <1197552751-53480-4-git-send-email-win@wincent.com>
Wincent Colaiuta wrote:
>
> - the emit_line_with_ws() function is also removed because that also
> rechecks the shitespace, and its functionality is rolled into ws.c
>
shitespace? It's fitting though. I'd stick with it :-)
--
Andreas Ericsson andreas.ericsson@op5.se
OP5 AB www.op5.se
Tel: +46 8-230225 Fax: +46 8-230231
^ permalink raw reply
* git config --get-regexp exit status
From: David Kågedal @ 2007-12-13 13:42 UTC (permalink / raw)
To: git
In-Reply-To: <20071213133653.13925.89254.stgit@krank>
David Kågedal <davidk@lysator.liu.se> writes:
> I made a patch that uses "git config --get-regexp" to get the
> description and stgit.stackformatversion options for all branches at
> once, and ignore the "protected" flag that I don't use. With this
> change, I'm almost down to half a second, which almost makes it
> usable.
One thing that annoyed me what that "git config --get-regexp" will
return zero, one, or more matches, which are all valid reponses. But
it treats the zero-match special and return an exit status of 1.
Is that a conscious choice, or just an effect of how "git config
--get" works?
Since zero matches isn't really an error, I would like the exit status
to be 0. At least for this use case :-)
--
David Kågedal
^ permalink raw reply
* [PATCH 2/5] New version of pre-commit hook
From: Wincent Colaiuta @ 2007-12-13 13:32 UTC (permalink / raw)
To: git; +Cc: gitster, Wincent Colaiuta
In-Reply-To: <1197552751-53480-2-git-send-email-win@wincent.com>
Now that "git diff --check" indicates problems with its exit code the
pre-commit hook becomes a trivial one-liner.
A side effect of this is that when "git diff --check" learns to detect
problems according to core.whitespace in the future, the hook's
behaviour will evolve to match without any changes required.
Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
templates/hooks--pre-commit | 67 ++++--------------------------------------
1 files changed, 7 insertions(+), 60 deletions(-)
diff --git a/templates/hooks--pre-commit b/templates/hooks--pre-commit
index 7092bae..f8c7be7 100644
--- a/templates/hooks--pre-commit
+++ b/templates/hooks--pre-commit
@@ -7,64 +7,11 @@
#
# To enable this hook, make this file executable.
-# This is slightly modified from Andrew Morton's Perfect Patch.
-# Lines you introduce should not have trailing whitespace.
-# Also check for an indentation that has SP before a TAB.
+git-diff-index -M --cached --check HEAD -- && exit 0
-if git-rev-parse --verify HEAD 2>/dev/null
-then
- git-diff-index -p -M --cached HEAD --
-else
- # NEEDSWORK: we should produce a diff with an empty tree here
- # if we want to do the same verification for the initial import.
- :
-fi |
-perl -e '
- my $found_bad = 0;
- my $filename;
- my $reported_filename = "";
- my $lineno;
- sub bad_line {
- my ($why, $line) = @_;
- if (!$found_bad) {
- print STDERR "*\n";
- print STDERR "* You have some suspicious patch lines:\n";
- print STDERR "*\n";
- $found_bad = 1;
- }
- if ($reported_filename ne $filename) {
- print STDERR "* In $filename\n";
- $reported_filename = $filename;
- }
- print STDERR "* $why (line $lineno)\n";
- print STDERR "$filename:$lineno:$line\n";
- }
- while (<>) {
- if (m|^diff --git a/(.*) b/\1$|) {
- $filename = $1;
- next;
- }
- if (/^@@ -\S+ \+(\d+)/) {
- $lineno = $1 - 1;
- next;
- }
- if (/^ /) {
- $lineno++;
- next;
- }
- if (s/^\+//) {
- $lineno++;
- chomp;
- if (/\s$/) {
- bad_line("trailing whitespace", $_);
- }
- if (/^\s* \t/) {
- bad_line("indent SP followed by a TAB", $_);
- }
- if (/^(?:[<>=]){7}/) {
- bad_line("unresolved merge conflict", $_);
- }
- }
- }
- exit($found_bad);
-'
+cat >&2 <<EOF
+fatal: commit aborted due to whitespace problems
+specify --no-verify to bypass these checks and commit anyway
+EOF
+
+exit 1
--
1.5.4.rc0.4.g50348
^ permalink raw reply related
* Re: [PATCH 2/2] parseopt: Add a gitcli(5) man page.
From: Pierre Habouzit @ 2007-12-13 13:56 UTC (permalink / raw)
To: Wincent Colaiuta; +Cc: Jeff King, git, Junio C Hamano
In-Reply-To: <A78DF383-1BD5-4AED-9DE8-60B8E96E54B5@wincent.com>
[-- Attachment #1: Type: text/plain, Size: 746 bytes --]
On jeu, déc 13, 2007 at 11:04:08 +0000, Wincent Colaiuta wrote:
> El 13/12/2007, a las 11:27, Pierre Habouzit escribió:
>
> >This page should hold every information about the git ways to parse
> >command
> >lines, and best practices to be used for scripting.
>
> Some feedback from a native English speaker follows...
FWIW, like always when it comes to documentation, you're welcome to
send a patch superseding my 2/2. I'm not a good English writer, I won't
be offended at all. I care about the page being here, not about it being
written by me.
--
·O· Pierre Habouzit
··O madcoder@debian.org
OOO http://www.madism.org
[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply
* [PATCH] git-gui - Unconditionally use absolute paths with Cygwin
From: Mark Levedahl @ 2007-12-13 14:02 UTC (permalink / raw)
To: spearce; +Cc: git, Mark Levedahl
The mapping of Windows vs Cygwin/Posix paths is arbitrary and not knowable
from the Posix side, so logic to determine whether to use relative paths
to locate the git-gui library just does not work on Cygwin. Fix this to
unconditionally use absolute path on Cygwin. (This fixes a regression
from 1.5.4).
Signed-off-by: Mark Levedahl <mdl123@verizon.net>
---
git-gui/Makefile | 10 ++++------
1 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/git-gui/Makefile b/git-gui/Makefile
index 26ac4b6..c109eab 100644
--- a/git-gui/Makefile
+++ b/git-gui/Makefile
@@ -117,14 +117,12 @@ GITGUI_SCRIPT := $$0
GITGUI_RELATIVE :=
GITGUI_MACOSXAPP :=
-ifeq ($(exedir),$(gg_libdir))
- GITGUI_RELATIVE := 1
-endif
-
ifeq ($(uname_O),Cygwin)
GITGUI_SCRIPT := `cygpath --windows --absolute "$(GITGUI_SCRIPT)"`
- ifeq ($(GITGUI_RELATIVE),)
- gg_libdir := $(shell cygpath --windows --absolute "$(gg_libdir)")
+ gg_libdir := $(shell cygpath --windows --absolute "$(gg_libdir)")
+else
+ ifeq ($(exedir),$(gg_libdir))
+ GITGUI_RELATIVE := 1
endif
endif
ifeq ($(uname_S),Darwin)
--
1.5.3.7.1167.g12f9a
^ permalink raw reply related
* Re: [PATCH] Teach git-gui to split hunks
From: Johannes Sixt @ 2007-12-13 14:03 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Junio C Hamano, Jason Sewall, Shawn O. Pearce, David,
Marco Costalba, Andy Parkins, git
In-Reply-To: <Pine.LNX.4.64.0712131248500.27959@racer.site>
Johannes Schindelin schrieb:
> On Thu, 13 Dec 2007, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>> For this to work, apply has to be called with --unidiff-zero, since
>>> the new hunks can start or stop with a "-" or "+" line.
>> You do not have to do "unidiff zero". Suppose you have this hunk you
>> need to split.
>>
>> [describes to pick zero or more '-' lines and zero or more '+' lines]
>
> I thought about that, but the UI is not trivial. The UI for my solution
> is.
It's probably sufficient to have an option "Stage this Line": Once you have
staged enough lines, the hunk will be split automatically by the current
number-of-context-lines setting.
-- Hannes
^ permalink raw reply
* Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call
From: Catalin Marinas @ 2007-12-13 14:04 UTC (permalink / raw)
To: David Kågedal; +Cc: git
In-Reply-To: <20071213133653.13925.89254.stgit@krank>
On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:
> I have a fair amount of branches, and I noticed that "stg branch -l"
> takes ridiculously long to finish.
I have the same problem.
> Maybe someone can help me find a quicker replacement for the
> get_protected call?
We can have the standard --list command which ignores the protected
flag or even the stgit.formatversion. Just a simple listing of the
branches (that's what I need most of the time). To get the
description, the 's' and 'p' flags, we could use --list-full or
something similar and wait a bit longer. This would also improve the
bash completion of commands taking branch names as arguments.
We had a similar issue in the past with 'series' as it was checking
whether the patch is empty. We ended up adding a '--empty' option for
this case.
--
Catalin
^ permalink raw reply
* Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call
From: David Kågedal @ 2007-12-13 14:15 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0712130604r6daf05d5n7afbadfe23831839@mail.gmail.com>
"Catalin Marinas" <catalin.marinas@gmail.com> writes:
> On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:
>> I have a fair amount of branches, and I noticed that "stg branch -l"
>> takes ridiculously long to finish.
>
> I have the same problem.
>
>> Maybe someone can help me find a quicker replacement for the
>> get_protected call?
>
> We can have the standard --list command which ignores the protected
> flag or even the stgit.formatversion. Just a simple listing of the
> branches (that's what I need most of the time). To get the
> description, the 's' and 'p' flags, we could use --list-full or
> something similar and wait a bit longer. This would also improve the
> bash completion of commands taking branch names as arguments.
If you remove everything but the branch name, you will have identical
output to "git branch", but much slower. Then there is no reason to
use it. The reason I used "stg branch -l" was that I found it
interesting to know which branches I had initialized with stgit, and I
could see uses for the description as well.
--
David Kågedal
^ permalink raw reply
* Re: [PATCH] Teach git-gui to split hunks
From: Johannes Schindelin @ 2007-12-13 14:18 UTC (permalink / raw)
To: Johannes Sixt
Cc: Junio C Hamano, Jason Sewall, Shawn O. Pearce, David,
Marco Costalba, Andy Parkins, git
In-Reply-To: <47613BA6.1060705@viscovery.net>
Hi,
On Thu, 13 Dec 2007, Johannes Sixt wrote:
> Johannes Schindelin schrieb:
> > On Thu, 13 Dec 2007, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >>
> >>> For this to work, apply has to be called with --unidiff-zero, since
> >>> the new hunks can start or stop with a "-" or "+" line.
> >> You do not have to do "unidiff zero". Suppose you have this hunk you
> >> need to split.
> >>
> >> [describes to pick zero or more '-' lines and zero or more '+' lines]
> >
> > I thought about that, but the UI is not trivial. The UI for my
> > solution is.
>
> It's probably sufficient to have an option "Stage this Line": Once you
> have staged enough lines, the hunk will be split automatically by the
> current number-of-context-lines setting.
And your hand falls off... ;-)
Seriously, I often have a big chunk of changes, for example with
an indentation change, where I want to stage everything _but_ one line in
the middle. Just staging that, "git checkout <file>" and fixing the
indentation of that single line speeds up my procedure vastly.
Ciao,
Dscho
^ permalink raw reply
* Re: [StGit RFC] Make "stg branch -l" faster by getting all git config information in one call
From: David Kågedal @ 2007-12-13 14:31 UTC (permalink / raw)
To: Catalin Marinas; +Cc: git
In-Reply-To: <b0943d9e0712130604r6daf05d5n7afbadfe23831839@mail.gmail.com>
"Catalin Marinas" <catalin.marinas@gmail.com> writes:
> On 13/12/2007, David Kågedal <davidk@lysator.liu.se> wrote:
>> I have a fair amount of branches, and I noticed that "stg branch -l"
>> takes ridiculously long to finish.
>
> I have the same problem.
>
>> Maybe someone can help me find a quicker replacement for the
>> get_protected call?
Hey, why not put the "protected" flag in the config? Then we can get
it the same way as the other stuff.
Protecting a branch is a configuration action, so it makes sense to
put it in the config.
--
David Kågedal
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox