* Re: [PATCH] Improve --patch option documentation in git-add (updated patch)
From: Nanako Shiraishi @ 2009-09-15 10:35 UTC (permalink / raw)
To: Jari Aalto; +Cc: Sean Estabrooks, Mikael Magnusson, git
In-Reply-To: <87tyz4k4eg.fsf@jondo.cante.net>
Quoting Jari Aalto <jari.aalto@cante.net>
> Nanako Shiraishi <nanako3@lavabit.com> writes:
>
>> Sorry, but this patch doesn't seem to apply anywhere. Have you fetched recently?
>
> Junio merged the patch at 5f2b1e6
Oh, I see.
If so, could you rebase and resend?
It would also be nicer if you followed Documentation/SubmittingPatches when composing your message, writing any additional comments after the three dashes line.
Thank you.
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Commited to wrong branch
From: Howard Miller @ 2009-09-15 10:31 UTC (permalink / raw)
To: git
Hi,
I am resurrecting a discussion from a week or two back (been on
holiday). As follows...
I had made some changes to some files and then done a commit. Only
then did I realise that I had the wrong branch checked out. To make
matters worse I then did a 'git reset HEAD^' which means that I can
now no longer switch branches. I am stuck. I had some advice (thanks!)
but it was not complete. I'd appreciate some more help.
I was advised to do a 'git reflog --branchname--' (I don't
know/understand what this command does) but it doesn't work. I just
get 'usage: git reflog (expire | ...)'
So basically I am no further forward. Just to reiterate I need to...
* remove the commit from my current branch (it tracks a remote so I
would prefer there to be no evidence to confuse other people after I
push)
* add the changes to the (other) branch they should have been added to.
* not loose or break anything.
Any (more) help appreciated.
^ permalink raw reply
* [PATCH 4/4] bash: teach 'git checkout' options
From: SZEDER Gábor @ 2009-09-15 10:21 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Junio C Hamano, SZEDER Gábor
In-Reply-To: <e927e4d3bfe50d93e5e6d65c46821158332b37f9.1253009868.git.szeder@ira.uka.de>
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
contrib/completion/git-completion.bash | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8c268a1..8e3cdbd 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -809,7 +809,18 @@ _git_checkout ()
{
__git_has_doubledash && return
- __gitcomp "$(__git_refs)"
+ local cur="${COMP_WORDS[COMP_CWORD]}"
+ case "$cur" in
+ --*)
+ __gitcomp "
+ --quiet --ours --theirs --track --no-track --merge
+ --conflict= --patch
+ "
+ ;;
+ *)
+ __gitcomp "$(__git_refs)"
+ ;;
+ esac
}
_git_cherry ()
--
1.6.5.rc1.92.gee3c1
^ permalink raw reply related
* [PATCH 3/4] bash: teach 'git reset --patch'
From: SZEDER Gábor @ 2009-09-15 10:21 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Junio C Hamano, SZEDER Gábor
In-Reply-To: <e927e4d3bfe50d93e5e6d65c46821158332b37f9.1253009868.git.szeder@ira.uka.de>
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
contrib/completion/git-completion.bash | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2529cec..8c268a1 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1782,7 +1782,7 @@ _git_reset ()
local cur="${COMP_WORDS[COMP_CWORD]}"
case "$cur" in
--*)
- __gitcomp "--merge --mixed --hard --soft"
+ __gitcomp "--merge --mixed --hard --soft --patch"
return
;;
esac
--
1.6.5.rc1.92.gee3c1
^ permalink raw reply related
* [PATCH 2/4] bash: update 'git stash' completion
From: SZEDER Gábor @ 2009-09-15 10:21 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Junio C Hamano, SZEDER Gábor
In-Reply-To: <e927e4d3bfe50d93e5e6d65c46821158332b37f9.1253009868.git.szeder@ira.uka.de>
This update adds 'git stash (apply|pop) --quiet' and all options known
to 'git stash save', and handles the DWIMery from 3c2eb80f (stash:
simplify defaulting to "save" and reject unknown options, 2009-08-18).
Care is taken to avoid offering subcommands in the DWIM case.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
contrib/completion/git-completion.bash | 20 ++++++++++++++++----
1 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c539385..2529cec 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1878,18 +1878,30 @@ _git_show_branch ()
_git_stash ()
{
+ local cur="${COMP_WORDS[COMP_CWORD]}"
+ local save_opts='--keep-index --no-keep-index --quiet --patch'
local subcommands='save list show apply clear drop pop create branch'
local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
- __gitcomp "$subcommands"
+ case "$cur" in
+ --*)
+ __gitcomp "$save_opts"
+ ;;
+ *)
+ if [ -z "$(__git_find_on_cmdline "$save_opts")" ]; then
+ __gitcomp "$subcommands"
+ else
+ COMPREPLY=()
+ fi
+ ;;
+ esac
else
- local cur="${COMP_WORDS[COMP_CWORD]}"
case "$subcommand,$cur" in
save,--*)
- __gitcomp "--keep-index"
+ __gitcomp "$save_opts"
;;
apply,--*|pop,--*)
- __gitcomp "--index"
+ __gitcomp "--index --quiet"
;;
show,--*|drop,--*|branch,--*)
COMPREPLY=()
--
1.6.5.rc1.92.gee3c1
^ permalink raw reply related
* [PATCH 1/4] bash: rename __git_find_subcommand() to __git_find_on_cmdline()
From: SZEDER Gábor @ 2009-09-15 10:21 UTC (permalink / raw)
To: Shawn O. Pearce; +Cc: git, Junio C Hamano, SZEDER Gábor
__git_find_subcommand() was originally meant to check whether
subcommands are already present on the command line. But the code is
general enough to be used for checking the presence of command line
options as well, and the next commit will use it for that purpose, so
let's give it a more general name.
Signed-off-by: SZEDER Gábor <szeder@ira.uka.de>
---
contrib/completion/git-completion.bash | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f47c519..c539385 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -619,8 +619,8 @@ __git_aliased_command ()
done
}
-# __git_find_subcommand requires 1 argument
-__git_find_subcommand ()
+# __git_find_on_cmdline requires 1 argument
+__git_find_on_cmdline ()
{
local word subcommand c=1
@@ -739,7 +739,7 @@ _git_bisect ()
__git_has_doubledash && return
local subcommands="start bad good skip reset visualize replay log run"
- local subcommand="$(__git_find_subcommand "$subcommands")"
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
return
@@ -1751,7 +1751,7 @@ _git_config ()
_git_remote ()
{
local subcommands="add rename rm show prune update set-head"
- local subcommand="$(__git_find_subcommand "$subcommands")"
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
return
@@ -1879,7 +1879,7 @@ _git_show_branch ()
_git_stash ()
{
local subcommands='save list show apply clear drop pop create branch'
- local subcommand="$(__git_find_subcommand "$subcommands")"
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
else
@@ -1910,7 +1910,7 @@ _git_submodule ()
__git_has_doubledash && return
local subcommands="add status init update summary foreach sync"
- if [ -z "$(__git_find_subcommand "$subcommands")" ]; then
+ if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then
local cur="${COMP_WORDS[COMP_CWORD]}"
case "$cur" in
--*)
@@ -1932,7 +1932,7 @@ _git_svn ()
proplist show-ignore show-externals branch tag blame
migrate
"
- local subcommand="$(__git_find_subcommand "$subcommands")"
+ local subcommand="$(__git_find_on_cmdline "$subcommands")"
if [ -z "$subcommand" ]; then
__gitcomp "$subcommands"
else
--
1.6.5.rc1.92.gee3c1
^ permalink raw reply related
* Re: [PATCH] Improve --patch option documentation in git-add (updated patch)
From: Jari Aalto @ 2009-09-15 8:17 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Sean Estabrooks, Mikael Magnusson, git
In-Reply-To: <20090915155208.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> Sorry, but this patch doesn't seem to apply anywhere. Have you fetched recently?
Junio merged the patch at 5f2b1e6
Jari
^ permalink raw reply
* Re: [PATCH 0/4] Colouring whitespace errors in diff -B output
From: Nanako Shiraishi @ 2009-09-15 6:52 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <1252995306-32329-1-git-send-email-gitster@pobox.com>
Quoting Junio C Hamano <gitster@pobox.com>
> Here is a 4-patch miniseries to teach "diff -B" output routines to detect
> and colour whitespace errors like we do for normal patches.
>
> The first three patches are only about moving code around without changing
> anything.
>
> The last one hooks "diff -B" logic to the per-line output routines in a
> way that mimicks how the normal patches are fed to them better, in order
> to take advantage of all the existing whitespace error detection and
> colouring logic.
>
> Junio C Hamano (4):
> diff.c: shuffling code around
> diff.c: split emit_line() from the first char and the rest of the line
> diff.c: emit_add_line() takes only the rest of the line
> diff -B: colour whitespace errors
>
> diff.c | 327 +++++++++++++++++++++++++++++++++++-----------------------------
> 1 files changed, 180 insertions(+), 147 deletions(-)
Sorry, but I don't seem to be able to apply these patches anywhere.
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: [PATCH] Improve --patch option documentation in git-add (updated patch)
From: Nanako Shiraishi @ 2009-09-15 6:52 UTC (permalink / raw)
To: Jari Aalto; +Cc: Sean Estabrooks, Mikael Magnusson, git
In-Reply-To: <87fxaolqhd.fsf_-_@jondo.cante.net>
Quoting Jari Aalto <jari.aalto@cante.net>
> Sean Estabrooks <seanlkml@sympatico.ca> writes:
>> ... To me though, it seems more difficult to parse this description
>> than the one offered by Junio in an earlier thread ...perhaps you'd
>> consider something closer to yours, such as:
>>
>> Interactively review the differences between the index and the
>> work tree and choose which hunks to add into the index.
>>
>> This effectively runs ``add --interactive``, but bypasses the
>> initial command menu and jumps directly to the `patch` subcommand.
>> See ``Interactive mode'' for details.
>
>
> Updated, thanks,
> Jari
>
>
> From be5eebc53c2e3dcf67edfb371d8aa8263e1a8d69 Mon Sep 17 00:00:00 2001
> From: Jari Aalto <jari.aalto@cante.net>
> Date: Tue, 15 Sep 2009 08:33:51 +0300
> Subject: [PATCH] Improve --patch option documentation in git-add
>
> Signed-off-by: Jari Aalto <jari.aalto@cante.net>
> ---
> Documentation/git-add.txt | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index e67b7e8..c57895a 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -72,9 +72,12 @@ OPTIONS
>
> -p::
> --patch::
> - Similar to Interactive mode but the initial command loop is
> - bypassed and the 'patch' subcommand is invoked using each of
> - the specified filepatterns before exiting.
> + Interactively review the differences between the index and the
> + work tree and choose which hunks to add into the index.
> +
> + This effectively runs ``add --interactive``, but bypasses the
> + initial command menu and jumps directly to the `patch` subcommand.
> + See ``Interactive mode'' for details.
>
> -e, \--edit::
> Open the diff vs. the index in an editor and let the user
Sorry, but this patch doesn't seem to apply anywhere. Have you fetched recently?
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* [PATCH 3/4] diff.c: emit_add_line() takes only the rest of the line
From: Junio C Hamano @ 2009-09-15 6:15 UTC (permalink / raw)
To: git; +Cc: Nanako Shiraishi
In-Reply-To: <1252995306-32329-1-git-send-email-gitster@pobox.com>
As the first character on the line that is fed to this function is always
"+", it is pointless to send that along with the rest of the line.
This change will make it easier to reuse the logic when emitting the
rewrite diff, as we do not want to copy a line only to add "+"/"-"/" "
immediately before its first character when we produce rewrite diff
output.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 14 ++++++++------
1 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/diff.c b/diff.c
index b5c2574..baf46ab 100644
--- a/diff.c
+++ b/diff.c
@@ -416,20 +416,22 @@ static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line
return ws_blank_line(line + 1, len - 1, ecbdata->ws_rule);
}
-static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
+static void emit_add_line(const char *reset,
+ struct emit_callback *ecbdata,
+ const char *line, int len)
{
const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
if (!*ws)
- emit_line(ecbdata->file, set, reset, line, len);
+ emit_line_0(ecbdata->file, set, reset, '+', line, len);
else if (new_blank_line_at_eof(ecbdata, line, len))
/* Blank line at EOF - paint '+' as well */
- emit_line(ecbdata->file, ws, reset, line, len);
+ emit_line_0(ecbdata->file, ws, reset, '+', line, len);
else {
/* Emit just the prefix, then the rest. */
- emit_line(ecbdata->file, set, reset, line, 1);
- ws_check_emit(line + 1, len - 1, ecbdata->ws_rule,
+ emit_line_0(ecbdata->file, set, reset, '+', "", 0);
+ ws_check_emit(line, len, ecbdata->ws_rule,
ecbdata->file, set, reset, ws);
}
}
@@ -726,7 +728,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
emit_line(ecbdata->file, color, reset, line, len);
} else {
ecbdata->lno_in_postimage++;
- emit_add_line(reset, ecbdata, line, len);
+ emit_add_line(reset, ecbdata, line + 1, len - 1);
}
}
--
1.6.5.rc1.54.g4aad
^ permalink raw reply related
* [PATCH 4/4] diff -B: colour whitespace errors
From: Junio C Hamano @ 2009-09-15 6:15 UTC (permalink / raw)
To: git; +Cc: Nanako Shiraishi
In-Reply-To: <1252995306-32329-1-git-send-email-gitster@pobox.com>
We used to send the old and new contents more or less straight out to the
output with only the original "old is red, new is green" colouring. Now
all the necessary support routines have been prepared, call them with a
line of data at a time from the output code and have them check and color
whitespace errors in exactly the same way as they are called from the low
level diff callback routines.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 75 +++++++++++++++++++++++++++++++++++++++++----------------------
1 files changed, 49 insertions(+), 26 deletions(-)
diff --git a/diff.c b/diff.c
index baf46ab..b6d40d7 100644
--- a/diff.c
+++ b/diff.c
@@ -296,28 +296,6 @@ static void print_line_count(FILE *file, int count)
}
}
-static void copy_file_with_prefix(FILE *file,
- int prefix, const char *data, int size,
- const char *set, const char *reset)
-{
- int ch, nl_just_seen = 1;
- while (0 < size--) {
- ch = *data++;
- if (nl_just_seen) {
- fputs(set, file);
- putc(prefix, file);
- }
- if (ch == '\n') {
- nl_just_seen = 1;
- fputs(reset, file);
- } else
- nl_just_seen = 0;
- putc(ch, file);
- }
- if (!nl_just_seen)
- fprintf(file, "%s\n\\ No newline at end of file\n", reset);
-}
-
static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
{
if (!DIFF_FILE_VALID(one)) {
@@ -436,6 +414,38 @@ static void emit_add_line(const char *reset,
}
}
+static void emit_rewrite_lines(struct emit_callback *ecb,
+ int prefix, const char *data, int size)
+{
+ const char *endp = NULL;
+ static const char *nneof = " No newline at end of file\n";
+ const char *old = diff_get_color(ecb->color_diff, DIFF_FILE_OLD);
+ const char *reset = diff_get_color(ecb->color_diff, DIFF_RESET);
+
+ while (0 < size) {
+ int len;
+
+ endp = memchr(data, '\n', size);
+ len = endp ? (endp - data + 1) : size;
+ if (prefix != '+') {
+ ecb->lno_in_preimage++;
+ emit_line_0(ecb->file, old, reset, '-',
+ data, len);
+ } else {
+ ecb->lno_in_postimage++;
+ emit_add_line(reset, ecb, data, len);
+ }
+ size -= len;
+ data += len;
+ }
+ if (!endp) {
+ const char *plain = diff_get_color(ecb->color_diff,
+ DIFF_PLAIN);
+ emit_line_0(ecb->file, plain, reset, '\\',
+ nneof, strlen(nneof));
+ }
+}
+
static void emit_rewrite_diff(const char *name_a,
const char *name_b,
struct diff_filespec *one,
@@ -447,10 +457,23 @@ static void emit_rewrite_diff(const char *name_a,
const char *name_a_tab, *name_b_tab;
const char *metainfo = diff_get_color(color_diff, DIFF_METAINFO);
const char *fraginfo = diff_get_color(color_diff, DIFF_FRAGINFO);
- const char *old = diff_get_color(color_diff, DIFF_FILE_OLD);
- const char *new = diff_get_color(color_diff, DIFF_FILE_NEW);
const char *reset = diff_get_color(color_diff, DIFF_RESET);
static struct strbuf a_name = STRBUF_INIT, b_name = STRBUF_INIT;
+ struct emit_callback ecbdata;
+
+ memset(&ecbdata, 0, sizeof(ecbdata));
+ ecbdata.color_diff = color_diff;
+ ecbdata.found_changesp = &o->found_changes;
+ ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
+ ecbdata.file = o->file;
+ if (ecbdata.ws_rule & WS_BLANK_AT_EOF) {
+ mmfile_t mf1, mf2;
+ fill_mmfile(&mf1, one);
+ fill_mmfile(&mf2, two);
+ check_blank_at_eof(&mf1, &mf2, &ecbdata);
+ }
+ ecbdata.lno_in_preimage = 1;
+ ecbdata.lno_in_postimage = 1;
name_a += (*name_a == '/');
name_b += (*name_b == '/');
@@ -475,9 +498,9 @@ static void emit_rewrite_diff(const char *name_a,
print_line_count(o->file, lc_b);
fprintf(o->file, " @@%s\n", reset);
if (lc_a)
- copy_file_with_prefix(o->file, '-', one->data, one->size, old, reset);
+ emit_rewrite_lines(&ecbdata, '-', one->data, one->size);
if (lc_b)
- copy_file_with_prefix(o->file, '+', two->data, two->size, new, reset);
+ emit_rewrite_lines(&ecbdata, '+', two->data, two->size);
}
struct diff_words_buffer {
--
1.6.5.rc1.54.g4aad
^ permalink raw reply related
* [PATCH 2/4] diff.c: split emit_line() from the first char and the rest of the line
From: Junio C Hamano @ 2009-09-15 6:15 UTC (permalink / raw)
To: git; +Cc: Nanako Shiraishi
In-Reply-To: <1252995306-32329-1-git-send-email-gitster@pobox.com>
A new helper function emit_line_0() takes the first line of diff output
(typically "-", " ", or "+") separately from the remainder of the line.
No other functional changes.
This change will make it easier to reuse the logic when emitting the
rewrite diff, as we do not want to copy a line only to add "+"/"-"/" "
immediately before its first character when we produce rewrite diff
output.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
diff.c | 10 +++++++++-
1 files changed, 9 insertions(+), 1 deletions(-)
diff --git a/diff.c b/diff.c
index 7548966..b5c2574 100644
--- a/diff.c
+++ b/diff.c
@@ -377,7 +377,8 @@ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
}
-static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len)
+static void emit_line_0(FILE *file, const char *set, const char *reset,
+ int first, const char *line, int len)
{
int has_trailing_newline, has_trailing_carriage_return;
@@ -389,6 +390,7 @@ static void emit_line(FILE *file, const char *set, const char *reset, const char
len--;
fputs(set, file);
+ fputc(first, file);
fwrite(line, len, 1, file);
fputs(reset, file);
if (has_trailing_carriage_return)
@@ -397,6 +399,12 @@ static void emit_line(FILE *file, const char *set, const char *reset, const char
fputc('\n', file);
}
+static void emit_line(FILE *file, const char *set, const char *reset,
+ const char *line, int len)
+{
+ emit_line_0(file, set, reset, line[0], line+1, len-1);
+}
+
static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
{
if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
--
1.6.5.rc1.54.g4aad
^ permalink raw reply related
* [PATCH 1/4] diff.c: shuffling code around
From: Junio C Hamano @ 2009-09-15 6:15 UTC (permalink / raw)
To: git; +Cc: Nanako Shiraishi
In-Reply-To: <1252995306-32329-1-git-send-email-gitster@pobox.com>
Move function, type, and structure definitions for fill_mmfile(),
count_trailing_blank(), check_blank_at_eof(), emit_line(),
new_blank_line_at_eof(), emit_add_line(), sane_truncate_fn, and
emit_callback up in the file, so that they can be refactored into helper
functions and reused by codepath for emitting rewrite patches.
This only moves the lines around to make the next two patches easier to
read.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This obviously comes on top of the earlier fix to the "blank lines at
eof" breakage.
diff.c | 250 ++++++++++++++++++++++++++++++++--------------------------------
1 files changed, 125 insertions(+), 125 deletions(-)
diff --git a/diff.c b/diff.c
index 63a3bfc..7548966 100644
--- a/diff.c
+++ b/diff.c
@@ -241,6 +241,23 @@ static struct diff_tempfile {
char tmp_path[PATH_MAX];
} diff_temp[2];
+typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
+
+struct emit_callback {
+ struct xdiff_emit_state xm;
+ int color_diff;
+ unsigned ws_rule;
+ int blank_at_eof_in_preimage;
+ int blank_at_eof_in_postimage;
+ int lno_in_preimage;
+ int lno_in_postimage;
+ sane_truncate_fn truncate;
+ const char **label_path;
+ struct diff_words_data *diff_words;
+ int *found_changesp;
+ FILE *file;
+};
+
static int count_lines(const char *data, int size)
{
int count, ch, completely_empty = 1, nl_just_seen = 0;
@@ -301,6 +318,114 @@ static void copy_file_with_prefix(FILE *file,
fprintf(file, "%s\n\\ No newline at end of file\n", reset);
}
+static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
+{
+ if (!DIFF_FILE_VALID(one)) {
+ mf->ptr = (char *)""; /* does not matter */
+ mf->size = 0;
+ return 0;
+ }
+ else if (diff_populate_filespec(one, 0))
+ return -1;
+ mf->ptr = one->data;
+ mf->size = one->size;
+ return 0;
+}
+
+static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)
+{
+ char *ptr = mf->ptr;
+ long size = mf->size;
+ int cnt = 0;
+
+ if (!size)
+ return cnt;
+ ptr += size - 1; /* pointing at the very end */
+ if (*ptr != '\n')
+ ; /* incomplete line */
+ else
+ ptr--; /* skip the last LF */
+ while (mf->ptr < ptr) {
+ char *prev_eol;
+ for (prev_eol = ptr; mf->ptr <= prev_eol; prev_eol--)
+ if (*prev_eol == '\n')
+ break;
+ if (!ws_blank_line(prev_eol + 1, ptr - prev_eol, ws_rule))
+ break;
+ cnt++;
+ ptr = prev_eol - 1;
+ }
+ return cnt;
+}
+
+static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
+ struct emit_callback *ecbdata)
+{
+ int l1, l2, at;
+ unsigned ws_rule = ecbdata->ws_rule;
+ l1 = count_trailing_blank(mf1, ws_rule);
+ l2 = count_trailing_blank(mf2, ws_rule);
+ if (l2 <= l1) {
+ ecbdata->blank_at_eof_in_preimage = 0;
+ ecbdata->blank_at_eof_in_postimage = 0;
+ return;
+ }
+ at = count_lines(mf1->ptr, mf1->size);
+ ecbdata->blank_at_eof_in_preimage = (at - l1) + 1;
+
+ at = count_lines(mf2->ptr, mf2->size);
+ ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
+}
+
+static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len)
+{
+ int has_trailing_newline, has_trailing_carriage_return;
+
+ has_trailing_newline = (len > 0 && line[len-1] == '\n');
+ if (has_trailing_newline)
+ len--;
+ has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
+ if (has_trailing_carriage_return)
+ len--;
+
+ fputs(set, file);
+ fwrite(line, len, 1, file);
+ fputs(reset, file);
+ if (has_trailing_carriage_return)
+ fputc('\r', file);
+ if (has_trailing_newline)
+ fputc('\n', file);
+}
+
+static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
+{
+ if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
+ ecbdata->blank_at_eof_in_preimage &&
+ ecbdata->blank_at_eof_in_postimage &&
+ ecbdata->blank_at_eof_in_preimage <= ecbdata->lno_in_preimage &&
+ ecbdata->blank_at_eof_in_postimage <= ecbdata->lno_in_postimage))
+ return 0;
+ return ws_blank_line(line + 1, len - 1, ecbdata->ws_rule);
+}
+
+static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
+{
+ const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
+ const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
+
+ if (!*ws)
+ emit_line(ecbdata->file, set, reset, line, len);
+ else if (new_blank_line_at_eof(ecbdata, line, len))
+ /* Blank line at EOF - paint '+' as well */
+ emit_line(ecbdata->file, ws, reset, line, len);
+ else {
+ /* Emit just the prefix, then the rest. */
+ emit_line(ecbdata->file, set, reset, line, 1);
+ ws_check_emit(line + 1, len - 1, ecbdata->ws_rule,
+ ecbdata->file, set, reset, ws);
+ }
+}
+
static void emit_rewrite_diff(const char *name_a,
const char *name_b,
struct diff_filespec *one,
@@ -345,20 +470,6 @@ static void emit_rewrite_diff(const char *name_a,
copy_file_with_prefix(o->file, '+', two->data, two->size, new, reset);
}
-static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
-{
- if (!DIFF_FILE_VALID(one)) {
- mf->ptr = (char *)""; /* does not matter */
- mf->size = 0;
- return 0;
- }
- else if (diff_populate_filespec(one, 0))
- return -1;
- mf->ptr = one->data;
- mf->size = one->size;
- return 0;
-}
-
struct diff_words_buffer {
mmfile_t text;
long alloc;
@@ -485,23 +596,6 @@ static void diff_words_show(struct diff_words_data *diff_words)
}
}
-typedef unsigned long (*sane_truncate_fn)(char *line, unsigned long len);
-
-struct emit_callback {
- struct xdiff_emit_state xm;
- int color_diff;
- unsigned ws_rule;
- int blank_at_eof_in_preimage;
- int blank_at_eof_in_postimage;
- int lno_in_preimage;
- int lno_in_postimage;
- sane_truncate_fn truncate;
- const char **label_path;
- struct diff_words_data *diff_words;
- int *found_changesp;
- FILE *file;
-};
-
static void free_diff_words_data(struct emit_callback *ecbdata)
{
if (ecbdata->diff_words) {
@@ -524,55 +618,6 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix)
return "";
}
-static void emit_line(FILE *file, const char *set, const char *reset, const char *line, int len)
-{
- int has_trailing_newline, has_trailing_carriage_return;
-
- has_trailing_newline = (len > 0 && line[len-1] == '\n');
- if (has_trailing_newline)
- len--;
- has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
- if (has_trailing_carriage_return)
- len--;
-
- fputs(set, file);
- fwrite(line, len, 1, file);
- fputs(reset, file);
- if (has_trailing_carriage_return)
- fputc('\r', file);
- if (has_trailing_newline)
- fputc('\n', file);
-}
-
-static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
-{
- if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
- ecbdata->blank_at_eof_in_preimage &&
- ecbdata->blank_at_eof_in_postimage &&
- ecbdata->blank_at_eof_in_preimage <= ecbdata->lno_in_preimage &&
- ecbdata->blank_at_eof_in_postimage <= ecbdata->lno_in_postimage))
- return 0;
- return ws_blank_line(line + 1, len - 1, ecbdata->ws_rule);
-}
-
-static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
-{
- const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
- const char *set = diff_get_color(ecbdata->color_diff, DIFF_FILE_NEW);
-
- if (!*ws)
- emit_line(ecbdata->file, set, reset, line, len);
- else if (new_blank_line_at_eof(ecbdata, line, len))
- /* Blank line at EOF - paint '+' as well */
- emit_line(ecbdata->file, ws, reset, line, len);
- else {
- /* Emit just the prefix, then the rest. */
- emit_line(ecbdata->file, set, reset, line, 1);
- ws_check_emit(line + 1, len - 1, ecbdata->ws_rule,
- ecbdata->file, set, reset, ws);
- }
-}
-
static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, unsigned long len)
{
const char *cp;
@@ -1464,51 +1509,6 @@ static const struct funcname_pattern_entry *diff_funcname_pattern(struct diff_fi
return NULL;
}
-static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)
-{
- char *ptr = mf->ptr;
- long size = mf->size;
- int cnt = 0;
-
- if (!size)
- return cnt;
- ptr += size - 1; /* pointing at the very end */
- if (*ptr != '\n')
- ; /* incomplete line */
- else
- ptr--; /* skip the last LF */
- while (mf->ptr < ptr) {
- char *prev_eol;
- for (prev_eol = ptr; mf->ptr <= prev_eol; prev_eol--)
- if (*prev_eol == '\n')
- break;
- if (!ws_blank_line(prev_eol + 1, ptr - prev_eol, ws_rule))
- break;
- cnt++;
- ptr = prev_eol - 1;
- }
- return cnt;
-}
-
-static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
- struct emit_callback *ecbdata)
-{
- int l1, l2, at;
- unsigned ws_rule = ecbdata->ws_rule;
- l1 = count_trailing_blank(mf1, ws_rule);
- l2 = count_trailing_blank(mf2, ws_rule);
- if (l2 <= l1) {
- ecbdata->blank_at_eof_in_preimage = 0;
- ecbdata->blank_at_eof_in_postimage = 0;
- return;
- }
- at = count_lines(mf1->ptr, mf1->size);
- ecbdata->blank_at_eof_in_preimage = (at - l1) + 1;
-
- at = count_lines(mf2->ptr, mf2->size);
- ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
-}
-
static void builtin_diff(const char *name_a,
const char *name_b,
struct diff_filespec *one,
--
1.6.5.rc1.54.g4aad
^ permalink raw reply related
* [PATCH 0/4] Colouring whitespace errors in diff -B output
From: Junio C Hamano @ 2009-09-15 6:15 UTC (permalink / raw)
To: git; +Cc: Nanako Shiraishi
Here is a 4-patch miniseries to teach "diff -B" output routines to detect
and colour whitespace errors like we do for normal patches.
The first three patches are only about moving code around without changing
anything.
The last one hooks "diff -B" logic to the per-line output routines in a
way that mimicks how the normal patches are fed to them better, in order
to take advantage of all the existing whitespace error detection and
colouring logic.
Junio C Hamano (4):
diff.c: shuffling code around
diff.c: split emit_line() from the first char and the rest of the line
diff.c: emit_add_line() takes only the rest of the line
diff -B: colour whitespace errors
diff.c | 327 +++++++++++++++++++++++++++++++++++-----------------------------
1 files changed, 180 insertions(+), 147 deletions(-)
^ permalink raw reply
* Re: [msysGit] [PATCH 07/17] Fix __stdcall/WINAPI placement and function prototype
From: Marius Storm-Olsen @ 2009-09-15 5:59 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Johannes Sixt, msysgit, git, lznuaa
In-Reply-To: <alpine.DEB.1.00.0909150224010.8306@pacific.mpi-cbg.de>
Johannes Schindelin said the following on 15.09.2009 02:24:
> Hi,
>
> On Mon, 14 Sep 2009, Johannes Sixt wrote:
>
>> On Montag, 14. September 2009, Marius Storm-Olsen wrote:
>>> WINAPI is a macro which translates into the proper calling convention, so
>>> replace __stdcall with that.
>> I've already pointed out elsewhere that the documentation of
>> _beginthreadex explicitly says that the calling convention of the
>> function pointer must be __stdcall. It does not mention WINAPI.
>> Therefore, I think that these two changes are not correct:
>>
>>> -static __stdcall unsigned ticktack(void *dummy)
>>> +static unsigned WINAPI ticktack(void *dummy)
>>> -static __stdcall unsigned run_thread(void *data)
>>> +static unsigned WINAPI run_thread(void *data)
>> You should s/WINAPI/__stdcall/.
>
> I don't think that comments by our most proficient MSys guy should be
> disregarded as thee comments were.
I also pointed out that WINAPI is just a define for __stdcall (since
_MSC_VER >= 800; VC 6.0 being _MSC_VER == 1200 and VC 2008 being
_MSC_VER == 1500), and that's probably never going to change now. So,
the change is not as bad as Hannes portrays it to be, and makes things
less convoluted by using the same convention all over.
However, I'll cave for the pressure, and reroll the commit :)
--
.marius
^ permalink raw reply
* Re: Patches for git-push --confirm and --show-subjects
From: Junio C Hamano @ 2009-09-15 5:50 UTC (permalink / raw)
To: Owen Taylor; +Cc: Daniel Barkalow, git
In-Reply-To: <1252982329.11581.111.camel@localhost.localdomain>
Owen Taylor <otaylor@redhat.com> writes:
> On Mon, 2009-09-14 at 17:46 -0700, Junio C Hamano wrote:
>> Owen Taylor <otaylor@redhat.com> writes:
>>
>> > If I can figure out the rest of it, I'll look at adding a hook on top as
>> > a sweetener :-)
>>
>> Please don't.
>>
>> I seriously suggest you start from, and stick to, nothing but a hook.
>>
>> The pre-push codepath is conceptually very simple --- something needs to
>> inspect a list of <ref, old, new> and say yes or no. But what the users
>> want needs great customizability (e.g. Daniel's sign-off validation
>> example). It's the prime example of codepath that should have a hook and
>> no built-in policy logic.
>
> Let me back up on this a little bit.
>
> Is confirmation a general need?
If you limit it to the confirmation alone, the answer is probably "not
necessarily". But a mechanism to allow validation logic to be plugged in
probably is.
You might not see a "policy" in your approach, but it makes some troubling
hardcoded policy decisions. Here are a few examples of what your patch
decides, and makes it harder for other people to build on (rather, "around):
- We support only interactive validation (confirmation). If you want to
have an unattended validation scheme, there is no way to enhance the
mechanism this patch adds to do so. You instead need to add yet
another command line option and hook into the same place as this patch
touches.
- We assume "git push" is run from terminal, and the only kind of
interactive validation we support is via typed confirmation from a line
terminal "[Y/n]?" If you want to run "git push" from a GUI frontend
and have the user interact with a dialog window popped up separately,
you are also out of luck.
- We assume it is good enough to have various built-in presentations of
supporting information while asking for confirmations; there is no way
for casual end users to customize and enhance it.
I honestly do not want to be a part of "We" in the above bullet points.
I do not object to having a good default presentation and default
interaction (assuming for a while that we limit ourselves only to
"interactive confirmation"). But that is a very different matter from
closing the door for other possibilities, which is essentially what the
approach to use built-in policy logic that is configurable with unbounded
number of future command line options to "git push" is.
> Providing a gnome-contributor-git-setup.sh is generally an approach of
> last resort.
No question about that. We do not have any complex built-in policy code
that is triggered at post-receive time at all, but many people use the
sample post-receive-email hook we ship unmodified in their repositories,
because the script is written in a highly configurable way. I do not see
why pre-push has to be any different.
In any case, this topic won't be part of 1.6.5, and we have plenty of time
to prototype and polish it before it goes to the end user.
^ permalink raw reply
* Re: [PATCH] Improve --patch option documentation in git-add (updated patch)
From: Jari Aalto @ 2009-09-15 5:35 UTC (permalink / raw)
To: Sean Estabrooks; +Cc: Mikael Magnusson, git
In-Reply-To: <BLU0-SMTP18292B09CCFD873F4A6DF6AEE40@phx.gbl>
[-- Attachment #1: Type: text/plain, Size: 565 bytes --]
Sean Estabrooks <seanlkml@sympatico.ca> writes:
> ... To me though, it seems more difficult to parse this description
> than the one offered by Junio in an earlier thread ...perhaps you'd
> consider something closer to yours, such as:
>
> Interactively review the differences between the index and the
> work tree and choose which hunks to add into the index.
>
> This effectively runs ``add --interactive``, but bypasses the
> initial command menu and jumps directly to the `patch` subcommand.
> See ``Interactive mode'' for details.
Updated, thanks,
Jari
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Improve-patch-option-documentation-in-git-add.patch --]
[-- Type: text/x-diff, Size: 1129 bytes --]
>From be5eebc53c2e3dcf67edfb371d8aa8263e1a8d69 Mon Sep 17 00:00:00 2001
From: Jari Aalto <jari.aalto@cante.net>
Date: Tue, 15 Sep 2009 08:33:51 +0300
Subject: [PATCH] Improve --patch option documentation in git-add
Signed-off-by: Jari Aalto <jari.aalto@cante.net>
---
Documentation/git-add.txt | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
index e67b7e8..c57895a 100644
--- a/Documentation/git-add.txt
+++ b/Documentation/git-add.txt
@@ -72,9 +72,12 @@ OPTIONS
-p::
--patch::
- Similar to Interactive mode but the initial command loop is
- bypassed and the 'patch' subcommand is invoked using each of
- the specified filepatterns before exiting.
+ Interactively review the differences between the index and the
+ work tree and choose which hunks to add into the index.
+
+ This effectively runs ``add --interactive``, but bypasses the
+ initial command menu and jumps directly to the `patch` subcommand.
+ See ``Interactive mode'' for details.
-e, \--edit::
Open the diff vs. the index in an editor and let the user
--
1.6.3.3
^ permalink raw reply related
* [PATCH] diff --whitespace: fix blank lines at end
From: Junio C Hamano @ 2009-09-15 5:05 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Yuri D'Elia, git
In-Reply-To: <7vvdjk95qi.fsf@alter.siamese.dyndns.org>
The earlier logic tried to colour any and all blank lines that were added
beyond the last blank line in the original, but this was very wrong. If
you added 96 blank lines, a non-blank line, and then 3 blank lines at the
end, only the last 3 lines should trigger the error, not the earlier 96
blank lines.
We need to also make sure that the lines are after the last non-blank line
in the postimage as well before deciding to paint them.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* This is _not_ about colouring whitespace errors in -B output;
that is a bit more involved change that needs a few more changes.
diff.c | 74 +++++++++++++++++++++++++++++++++-------------
t/t4019-diff-wserror.sh | 2 +-
2 files changed, 54 insertions(+), 22 deletions(-)
diff --git a/diff.c b/diff.c
index 2b285b8..63a3bfc 100644
--- a/diff.c
+++ b/diff.c
@@ -491,8 +491,10 @@ struct emit_callback {
struct xdiff_emit_state xm;
int color_diff;
unsigned ws_rule;
- int blank_at_eof;
+ int blank_at_eof_in_preimage;
+ int blank_at_eof_in_postimage;
int lno_in_preimage;
+ int lno_in_postimage;
sane_truncate_fn truncate;
const char **label_path;
struct diff_words_data *diff_words;
@@ -542,6 +544,17 @@ static void emit_line(FILE *file, const char *set, const char *reset, const char
fputc('\n', file);
}
+static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
+{
+ if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
+ ecbdata->blank_at_eof_in_preimage &&
+ ecbdata->blank_at_eof_in_postimage &&
+ ecbdata->blank_at_eof_in_preimage <= ecbdata->lno_in_preimage &&
+ ecbdata->blank_at_eof_in_postimage <= ecbdata->lno_in_postimage))
+ return 0;
+ return ws_blank_line(line + 1, len - 1, ecbdata->ws_rule);
+}
+
static void emit_add_line(const char *reset, struct emit_callback *ecbdata, const char *line, int len)
{
const char *ws = diff_get_color(ecbdata->color_diff, DIFF_WHITESPACE);
@@ -549,11 +562,8 @@ static void emit_add_line(const char *reset, struct emit_callback *ecbdata, cons
if (!*ws)
emit_line(ecbdata->file, set, reset, line, len);
- else if ((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
- ecbdata->blank_at_eof &&
- (ecbdata->blank_at_eof <= ecbdata->lno_in_preimage) &&
- ws_blank_line(line + 1, len - 1, ecbdata->ws_rule))
- /* Blank line at EOF */
+ else if (new_blank_line_at_eof(ecbdata, line, len))
+ /* Blank line at EOF - paint '+' as well */
emit_line(ecbdata->file, ws, reset, line, len);
else {
/* Emit just the prefix, then the rest. */
@@ -581,12 +591,19 @@ static unsigned long sane_truncate_line(struct emit_callback *ecb, char *line, u
return allot - l;
}
-static int find_preimage_lno(const char *line)
+static void find_lno(const char *line, struct emit_callback *ecbdata)
{
- char *p = strchr(line, '-');
+ const char *p;
+ ecbdata->lno_in_preimage = 0;
+ ecbdata->lno_in_postimage = 0;
+ p = strchr(line, '-');
if (!p)
- return 0; /* should not happen */
- return strtol(p+1, NULL, 10);
+ return; /* cannot happen */
+ ecbdata->lno_in_preimage = strtol(p + 1, NULL, 10);
+ p = strchr(p, '+');
+ if (!p)
+ return; /* cannot happen */
+ ecbdata->lno_in_postimage = strtol(p + 1, NULL, 10);
}
static void fn_out_consume(void *priv, char *line, unsigned long len)
@@ -613,7 +630,7 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
if (line[0] == '@') {
len = sane_truncate_line(ecbdata, line, len);
- ecbdata->lno_in_preimage = find_preimage_lno(line);
+ find_lno(line, ecbdata);
emit_line(ecbdata->file,
diff_get_color(ecbdata->color_diff, DIFF_FRAGINFO),
reset, line, len);
@@ -651,10 +668,13 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
diff_get_color(ecbdata->color_diff,
line[0] == '-' ? DIFF_FILE_OLD : DIFF_PLAIN);
ecbdata->lno_in_preimage++;
+ if (line[0] == ' ')
+ ecbdata->lno_in_postimage++;
emit_line(ecbdata->file, color, reset, line, len);
- return;
+ } else {
+ ecbdata->lno_in_postimage++;
+ emit_add_line(reset, ecbdata, line, len);
}
- emit_add_line(reset, ecbdata, line, len);
}
static char *pprint_rename(const char *a, const char *b)
@@ -1470,16 +1490,23 @@ static int count_trailing_blank(mmfile_t *mf, unsigned ws_rule)
return cnt;
}
-static int adds_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2, unsigned ws_rule)
+static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
+ struct emit_callback *ecbdata)
{
int l1, l2, at;
+ unsigned ws_rule = ecbdata->ws_rule;
l1 = count_trailing_blank(mf1, ws_rule);
l2 = count_trailing_blank(mf2, ws_rule);
- if (l2 <= l1)
- return 0;
- /* starting where? */
+ if (l2 <= l1) {
+ ecbdata->blank_at_eof_in_preimage = 0;
+ ecbdata->blank_at_eof_in_postimage = 0;
+ return;
+ }
at = count_lines(mf1->ptr, mf1->size);
- return (at - l1) + 1; /* the line number counts from 1 */
+ ecbdata->blank_at_eof_in_preimage = (at - l1) + 1;
+
+ at = count_lines(mf2->ptr, mf2->size);
+ ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
}
static void builtin_diff(const char *name_a,
@@ -1572,8 +1599,7 @@ static void builtin_diff(const char *name_a,
ecbdata.found_changesp = &o->found_changes;
ecbdata.ws_rule = whitespace_rule(name_b ? name_b : name_a);
if (ecbdata.ws_rule & WS_BLANK_AT_EOF)
- ecbdata.blank_at_eof =
- adds_blank_at_eof(&mf1, &mf2, ecbdata.ws_rule);
+ check_blank_at_eof(&mf1, &mf2, &ecbdata);
ecbdata.file = o->file;
xpp.flags = XDF_NEED_MINIMAL | o->xdl_opts;
xecfg.ctxlen = o->context;
@@ -1699,7 +1725,13 @@ static void builtin_checkdiff(const char *name_a, const char *name_b,
xdi_diff(&mf1, &mf2, &xpp, &xecfg, &ecb);
if (data.ws_rule & WS_BLANK_AT_EOF) {
- int blank_at_eof = adds_blank_at_eof(&mf1, &mf2, data.ws_rule);
+ struct emit_callback ecbdata;
+ int blank_at_eof;
+
+ ecbdata.ws_rule = data.ws_rule;
+ check_blank_at_eof(&mf1, &mf2, &ecbdata);
+ blank_at_eof = ecbdata.blank_at_eof_in_preimage;
+
if (blank_at_eof) {
static char *err;
if (!err)
diff --git a/t/t4019-diff-wserror.sh b/t/t4019-diff-wserror.sh
index 1e75f1a..3a3663f 100755
--- a/t/t4019-diff-wserror.sh
+++ b/t/t4019-diff-wserror.sh
@@ -193,7 +193,7 @@ test_expect_success 'do not color trailing cr in context' '
test_expect_success 'color new trailing blank lines' '
{ echo a; echo b; echo; echo; } >x &&
git add x &&
- { echo a; echo; echo; echo; echo; } >x &&
+ { echo a; echo; echo; echo; echo c; echo; echo; echo; echo; } >x &&
git diff --color x >output &&
cnt=$(grep "${blue_grep}" output | wc -l) &&
test $cnt = 2
--
1.6.5.rc1.54.g4aad
^ permalink raw reply related
* Re: [Bug?] "diff -B --color" output doesn't show space errors
From: Junio C Hamano @ 2009-09-15 4:44 UTC (permalink / raw)
To: Nanako Shiraishi; +Cc: Yuri D'Elia, git
In-Reply-To: <20090915123456.6117@nanako3.lavabit.com>
Nanako Shiraishi <nanako3@lavabit.com> writes:
> I wanted to try this -B option, and wrote a little test program.
>
> While it shows correctly that the file was rewritten, it doesn't
> point out various whitespace mistakes in the file anymore.
As you included the trailing blank lines, I assume you are running
'next'.
The code to emit complete rewrite patch hasn't changed much since it was
written, and I do not think it is aware of any whitespace error checking,
let alone the "trailing blank lines", which is pretty new for even the
regular diff codepath.
But a more interesting thing about your test program is that it exposes to
a bug in the new code in next. Let me cook up a patch to fix that issue
first, and then build probably a few more patches on it to add whitespace
error highlighting to the complete-rewrite codepath.
^ permalink raw reply
* Re: [PATCH 4/4] reset: add test cases for "--merge-dirty" option
From: Christian Couder @ 2009-09-15 4:32 UTC (permalink / raw)
To: Daniel Barkalow
Cc: Junio C Hamano, git, Johannes Schindelin, Stephan Beyer,
Jakub Narebski, Linus Torvalds
In-Reply-To: <alpine.LNX.2.00.0909110120520.28290@iabervon.org>
On Friday 11 September 2009, Daniel Barkalow wrote:
> On Fri, 11 Sep 2009, Christian Couder wrote:
> > On Friday 11 September 2009, Daniel Barkalow wrote:
> > > On Thu, 10 Sep 2009, Christian Couder wrote:
> > >
> > > This shows that with the "--merge-dirty" option,
> > >
> > > changes that are both in the work tree and the index are kept
> > >
> > > in the work tree after the reset (but discarded in the index). As
> > > with the "--merge" option,
> > >
> > > changes that are in both the work tree and the index are discarded
> > >
> > > after the reset.
> > >
> > > I'm lost here.
> > >
> > > If you have:
> > >
> > > working index HEAD target
> > > version B B A A
> > >
> > > You get:
> > >
> > > working index HEAD target
> > > --m-d B A A A
> > > --merge A A A A
> > >
> > > ?
> >
> > Yes, files that are not different between HEAD and target are changed
> > like that. Thanks for explaining it better than I could!
>
> I worked on the rules for merging way back when, so I've looked at tables
> of cases like that. If there are more cases to cover, it might work
> better to have a table like:
>
> working index HEAD target working index HEAD
> B B A A --m-d B A A
> --merge A A A
> B B A C --m-d (disallowed)
> --merge C C C
>
> Are there other differences?
Yes, I found that I messed up the last test in patch 4/4. I forgot to
replace some --merge with --merge-dirty :-(
In fact while "reset --merge" fails when there are changes in files that are
changed between HEAD and target, "reset --merge-dirty" will not fail and
discard these changes. So it is not really safe in this case and I am
working on trying to make it safer in this case.
> > > > ---
> > > > t/t7110-reset-merge.sh | 54
> > > > +++++++++++++++++++++++++++++++++++++++++++---- 1 files changed, 49
> > > > insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/t/t7110-reset-merge.sh b/t/t7110-reset-merge.sh
> > > > index 45714ae..1e6d634 100755
> > > > --- a/t/t7110-reset-merge.sh
> > > > +++ b/t/t7110-reset-merge.sh
> > > > @@ -19,7 +19,7 @@ test_expect_success 'creating initial files' '
> > > > git commit -m "Initial commit"
> > > > '
> > > >
> > > > -test_expect_success 'ok with changes in file not changed by reset'
> > > > ' +test_expect_success '--merge: ok if changes in file not touched
> > > > by reset' '
> > >
> > > Should probably have the "--merge: " from the beginning, since you're
> > > adding the test in this series anyway. That would make the diff come
> > > out clearer.
> >
> > Yeah, but I am not sure that patches 3/4 and 4/4 will get merged in the
> > end. If they are not merged it will be better if there is no "--merge:
> > ".
>
> Maybe write those lines to mention "reset --merge" naturally? Like:
>
> 'ok with changes in file not changed by reset --merge'
>
> 'reset --merge discards changes added to index 1'
Ok I will do that.
Thanks,
Christian.
^ permalink raw reply
* [Bug?] "diff -B --color" output doesn't show space errors
From: Nanako Shiraishi @ 2009-09-15 3:34 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Yuri D'Elia, git
In-Reply-To: <7viqfmsoej.fsf@alter.siamese.dyndns.org>
Quoting Junio C Hamano <gitster@pobox.com>
> By default, if the pathname that was present in the old version still
> appears in the new version, that path is not considered as a candiate
> for rename detection. Only "X used to be there but is gone" and "Y did
> not exist but appeared" are paired up and checked if they are similar.
>
> Give the command -B option, too, to break the filepair that does not
> disappear.
I wanted to try this -B option, and wrote a little test program.
While it shows correctly that the file was rewritten, it doesn't
point out various whitespace mistakes in the file anymore.
Is this a bug, or should I give some other options as well?
-- >8 -- cut here -- 8< --
git init
cat >file <<"EOF"
This is an article that will be
completely rewritten in a
later commit.
EOF
git add file
sed -e "s/T/\t/g" -e "s/_/ /g" >file <<"EOF"
An article was written but it was_
later rewritten to be_
a completely different text.
_____
An article was written but it was_
later rewritten to be_
a completely different text.
An article was written but it was_
later rewritten to be_
a completely different text.
Worse yet, the replacement text_
introduces a lot of
_Twhite space errors_
such as SP before HT and trailing
whitespaces, when the file was modified by the
later commit.
Also there are trailing empty lines at the end of the file.
EOF
git diff --color
git diff --color -B
# end of test program
--
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/
^ permalink raw reply
* Re: Patches for git-push --confirm and --show-subjects
From: Owen Taylor @ 2009-09-15 2:38 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Daniel Barkalow, git
In-Reply-To: <7v7hw19gr5.fsf@alter.siamese.dyndns.org>
On Mon, 2009-09-14 at 17:46 -0700, Junio C Hamano wrote:
> Owen Taylor <otaylor@redhat.com> writes:
>
> > If I can figure out the rest of it, I'll look at adding a hook on top as
> > a sweetener :-)
>
> Please don't.
>
> I seriously suggest you start from, and stick to, nothing but a hook.
>
> The pre-push codepath is conceptually very simple --- something needs to
> inspect a list of <ref, old, new> and say yes or no. But what the users
> want needs great customizability (e.g. Daniel's sign-off validation
> example). It's the prime example of codepath that should have a hook and
> no built-in policy logic.
Let me back up on this a little bit.
Is confirmation a general need?
In the context of the kernel or git personal repository workflows,
probably not. If you push something wrong, and discover it quickly, you
can just push over it and nobody is wiser. But a large fraction of the
projects listed on the front page of git-scm.com are using shared
repositories. And with a shared repository, a messed up push is more of
an issue: there may be notifications sent out over email or IRC, the
repository may be configured with denyFastForward true, people may
quickly pull your accidental push, etc.
It's also a sticky point for first using git. The push syntax and
behavior is a bit cryptic until you are used to it. Is it going to push
all branches or just the one I'm on? Is 'git push --tags' a superset of
'git push'? etc. If the first repository you are pushing to is public
and shared, heavy use of --dry-run at first is certainly advisable. But
repeating with --dry-run and without is pretty awkward.
How would the quality of use be as a hook?
Probably good enough. The broad outlines are achievable anyways. There
are some aspects of my patches that wouldn't be there. A few that come
to mind:
- The --show-subjects option applied to all displays of push
references, not just for --confirm.
- In the case of a successful push when the updates are exactly what
was confirmed, outputting them again after the push is suppressed.
How would ease of configuration be for a hook?
> E.g. perhaps in $HOME/.gitconfig, you may want to allow
>
> [hook]
> prePush = $HOME/.githooks/my-pre-push-hook
> preCommit = $HOME/.githooks/my-pre-commit-hook
This is certainly better than having to set it up per-repo, but if I
wanted to tell GNOME contributors how to turn it on, I'd have to provide
a gnome-contributor-git-setup.sh. Even if the hooks were shipped with
git, there's not going to be a cross-distro path to the where they are
installed.
Maybe if a there was a "hook path" that included ~/.githooks and a
system directory? Though:
git-config --global hook.prePush git-pre-push-confirm
could still overwrite something that they already have configured; it
wouldn't be an "orthogonal tip" that you could find on a web page and
apply blindly.
Providing a gnome-contributor-git-setup.sh is generally an approach of
last resort. I don't think there is anything unique or special about how
we do we do git on gnome.org that makes it different from other
shared-repository workflows. I'd like the knowledge that people get
using Git with GNOME to carry over to other work they do with Git and
vice-versa.
- Owen
^ permalink raw reply
* Re: Patches for git-push --confirm and --show-subjects
From: Daniel Barkalow @ 2009-09-15 0:55 UTC (permalink / raw)
To: Owen Taylor; +Cc: Junio C Hamano, git
In-Reply-To: <1252970294.11581.71.camel@localhost.localdomain>
On Mon, 14 Sep 2009, Owen Taylor wrote:
> On Mon, 2009-09-14 at 18:21 -0400, Daniel Barkalow wrote:
> > On Sun, 13 Sep 2009, Owen Taylor wrote:
>
> [...]
> > I think the classification logic should move to match_refs(), assuming you
> > mean the ref->nonfastforward and ref->deletion stuff. It would probably
> > also be worth having a bit for "already up to date". (Note that
> > cmd_send_pack() calls match_refs(), so there wouldn't have to be
> > duplication between the legacy cmd_send_pack() code path and the
> > transport_push() codepath if the code moved into match_refs()).
> [...]
> > > - You add another vfunc to the transport - '->get_capabilities' or
> > > something - that encapsulates server_supports("delete-refs").
> >
> > I think it would be better to have a vfunc that takes refs with the
> > classification bits set and sets the statuses based on the idea that we're
> > not going to lose any races and the remote won't reject our change for
> > some reason we don't know about. There's a potentially large and varied
> > set of restrictions on what the other side is willing to accept, and I
> > think it would be better to put that on the other side of the vfunc,
> > rather than having the main transport code know that "delete-refs" means
> > that you can delete refs, "nonfastforward" means you can force a
> > non-fast-forward, something means you can create files named "CVS", etc.
>
> match_refs seems like a reasonable place to put this logic, but I'm not
> sure I completely follow what you are proposing in terms of
> transport-specific customization.
>
> match_refs() is called from a couple of places where there is no
> 'transport' (cmd_send_pack() and http-push.c) so it can't itself call
> into the transport code.
>
> Are you thinking of a virtual function that would be a "second pass"
> after the main logic done is done by match_refs; so ->check_refs()
> virtual function?
Yes. match_refs() would answer the question of what the change to the ref
is, while ->check_refs() would determine whether, for this transport, that
change is possible. transport_push() would call match_refs(), then
->check_refs() for transport-specific limitations, then local "are you
sure" checks, then push_refs(). Other places that call match_refs() would
either call the appropriate implementation of check_refs()
(e.g., from cmd_send_pack) or just let the change get rejected when it is
actually attempted.
> How would the 'bit for "already up to date"' differ from
> REF_STATUS_UPTODATE. ?
It would put all of the things that match_refs() generated in the
collection of 1-bit flags, and leave ->status entirely for the
transport-specific code to set. Possibly transport_push() should set
status to REF_STATUS_UPTODATE if the bit is set, and similarly set status
to REF_STATUS_REJECT_NONFASTFORWARD if nonfastforward and not force.
> > I think a pre-push hook would be popular; I know I'd like to have a hook
> > that makes sure that I signed off anything I'm pushing (when the server
> > might check that *somebody* did, but wouldn't know that this push is
> > supposed to be me), and I'd like a hook that checks that I've referenced
> > an issue in an issue tracker for each commit that I'm pushing (but only
> > when I go to push it).
>
> If I can figure out the rest of it, I'll look at adding a hook on top as
> a sweetener :-)
Sounds like a good plan.
-Daniel
*This .sig left intentionally blank*
^ permalink raw reply
* Re: Patches for git-push --confirm and --show-subjects
From: Junio C Hamano @ 2009-09-15 0:46 UTC (permalink / raw)
To: Owen Taylor; +Cc: Daniel Barkalow, git
In-Reply-To: <1252970294.11581.71.camel@localhost.localdomain>
Owen Taylor <otaylor@redhat.com> writes:
> If I can figure out the rest of it, I'll look at adding a hook on top as
> a sweetener :-)
Please don't.
I seriously suggest you start from, and stick to, nothing but a hook.
The pre-push codepath is conceptually very simple --- something needs to
inspect a list of <ref, old, new> and say yes or no. But what the users
want needs great customizability (e.g. Daniel's sign-off validation
example). It's the prime example of codepath that should have a hook and
no built-in policy logic.
You have to enable the necessary hook in all your repositories, and if
that bothers you, then *that* can (and should) be solved as a separate
issue by devising a mechanism that can be extended to the other hooks to
solve the same issue once and for all.
E.g. perhaps in $HOME/.gitconfig, you may want to allow
[hook]
prePush = $HOME/.githooks/my-pre-push-hook
preCommit = $HOME/.githooks/my-pre-commit-hook
Lack of a general mechanism to allow users to say "I want this hook to
apply to all of my repositories" is not an excuse to add tons of complex
code in the codepath. Just give users the mechanism and leave the policy
logic to them.
^ permalink raw reply
* Re: [msysGit] [PATCH 07/17] Fix __stdcall/WINAPI placement and function prototype
From: Johannes Schindelin @ 2009-09-15 0:24 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Marius Storm-Olsen, msysgit, git, lznuaa
In-Reply-To: <200909142200.52174.j6t@kdbg.org>
Hi,
On Mon, 14 Sep 2009, Johannes Sixt wrote:
> On Montag, 14. September 2009, Marius Storm-Olsen wrote:
> > WINAPI is a macro which translates into the proper calling convention, so
> > replace __stdcall with that.
>
> I've already pointed out elsewhere that the documentation of
> _beginthreadex explicitly says that the calling convention of the
> function pointer must be __stdcall. It does not mention WINAPI.
> Therefore, I think that these two changes are not correct:
>
> > -static __stdcall unsigned ticktack(void *dummy)
> > +static unsigned WINAPI ticktack(void *dummy)
>
> > -static __stdcall unsigned run_thread(void *data)
> > +static unsigned WINAPI run_thread(void *data)
>
> You should s/WINAPI/__stdcall/.
I don't think that comments by our most proficient MSys guy should be
disregarded as thee comments were.
Ciao,
Dscho
^ 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