Re: [RFC/PATCH 2/2] WIP xdiff: markup duplicates differently

[Stefan Beller]

On Sat, Sep 3, 2016 at 5:25 AM, Jakub Narębski  wrote:
> W dniu 03.09.2016 o 05:31, Stefan Beller pisze:
>
>> When moving code (e.g. a function is moved to another part of the file or
>> to a different file), the review process is different than reviewing new
>> code. When reviewing moved code we are only interested in the diff as
>> where there are differences in the moved code, e.g. namespace changes.
>>
>> However the inner part of these moved texts should not change.
>> To aid a developer reviewing such code, emit it with a different prefix
>> than the usual +,- to indicate it is overlapping code.
>
> What would be this different prefix?

I will discard the part of the different prefix as the design of 2/2
will change.



>
>
> Side note: I wonder if the cousin of unified diff, namely context diff[1],
> is something that we can and should support.



>
> [1]: 
> https://www.gnu.org/software/diffutils/manual/html_node/Context-Format.html
>  
> https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Context.html
>
> *** lao 2002-02-21 23:30:39.942229878 -0800
> --- tzu 2002-02-21 23:30:50.442260588 -0800
> ***
> *** 1,7 
> - The Way that can be told of is not the eternal Way;
> - The name that can be named is not the eternal name.
>   The Nameless is the origin of Heaven and Earth;
> ! The Named is the mother of all things.
>   Therefore let there always be non-being,
> so we may see their subtlety,
>   And let there always be being,
> --- 1,6 
>   The Nameless is the origin of Heaven and Earth;
> ! The named is the mother of all things.
> !

So the line moved here?
Is it intentional that the line differs though?
(capitalisation of 'named")
Not sure I can read this diff correctly.

I think for this small side project I'd rather want
to 'just' support colors of moved code;)

>   Therefore let there always be non-being,
> so we may see their subtlety,
>   And let there always be being,
> ***
> *** 9,11 
> --- 8,13 
>   The two are the same,
>   But after they are produced,
> they have different names.
> + They both may be called deep and profound.
> + Deeper and more profound,
> + The door of all subtleties!

Re: [RFC/PATCH 0/2] Color moved code differently

[Stefan Beller]

On Sat, Sep 3, 2016 at 12:00 AM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> A line is colored differently if that line and the surroundign 2 lines
>> appear as-is in the opposite part of the diff.
>>
>> Example:
>> http://i.imgur.com/ay84q0q.png
>>
>> Or apply these patches and
>> git show e28eae3184b26d3cf3293e69403babb5c575342c
>> git show bc9204d4ef6e0672389fdfb0d398fa9a39dba3d5
>> git show 8465541e8ce8eaf16e66ab847086779768c18f2d
>
> I like this as a concept.  Two quick comments are

Great!

>
>  * On 1/2, you would also need to teach diff-color-slot the
>correspondence between the name used by configuration and the
>enum used as the index into the diff_colors[] array.

So I would need to add code to diff_parse_color_slot just below the
definition of the struct.

>I think
>these are not "DUPLICATE", but "MOVE", so I'd suggest renaming
>dup-new and dup-old to some words or phrases that have "MOVED"
>and "TO" or "FROM" in them (e.g. "DIFF_MOVED_FROM",
>"DIFF_MOVED_TO").

Ok, sounds sensible.

>
>  * On 2/2, doing it at xdiff.c level may be limiting this good idea
>to flourish to its full potential, as the interface is fed only
>one diff_filepair at a time.

I realized that after I implemented it. I agree we would want to have
it function cross file.

So from my current understanding of the code,
* diffcore_std would call a new function diffcore_detect_moved(void)
   just before diffcore_apply_filter is called.
* The new function diffcore_detect_moved would then check if the
   diff is a valid textual diff (i.e. real files, not submodules, but
   deletion/creation of one file is allowed)
   If so we generate the diff internally and as in 2/2 would
   hash all added/removed lines with context and store it.
* Instead checking for a different symbol in fn_out_consume, we consult
   the hashmap whether we want to color the line as a "moved" line.

> All the examples you pointed at
>above have line movement within a single path because of this
>design limitation.  I do not think 2/2 would serve as a small but
>good first step to build on top of to enhance the feature to
>notice line movements across files and the design (not the
>implementation) needs rethinking.

After reading the code more closely I agree. I initially put it there
to see if it is feasible or just messing up the diff. And as I have
a bit of knowledge of the xdl internals due to the first version of
the diff slider heuristic, I went with that.

>From a cursory look at the history, you seemed to be very involved in
the implementation of diff.c, so I'd appreciate strong guidance on
the design level.

>
> The idea has a potential to help reviewing inter-file movement of
> lines in 3b0c4200 ("apply: move libified code from builtin/apply.c
> to apply.{c,h}", 2016-08-08).  You can see what was _changed_ in the
> part that has been moved across files with "show -B -M", and
> sometimes that is useful, but at the same time, you cannot see what
> was moved without changing, which often is necessary to understand
> the changes and notice things like "you moved this across files
> without changing, but this and that you did not change need to be
> adjusted".
>
> The coloring of "these are moved verbatim" in the style this series
> gives would be very helpful for reviewing such a change.
>

Right, that is what triggered me to implement it.
Another such case that I reviewed was
https://git.eclipse.org/r/#/c/78645/
and I felt very insecure about reviewing thousands lines of code
that was "just moved".

Thanks,
Stefan

I want this app removed

[3344688792]

I want this app removed

Re: [PATCH] stash: allow ref of a stash by index

[Jeff King]

On Sat, Sep 03, 2016 at 07:21:18PM -0400, Aaron M Watson wrote:

> Allows stashes to be referenced by index only. Instead of referencing
> "stash@{n}" explicitly, it can simply be referenced as "n".

This says "what" but not "why". I assume it is "because the former is
more annoying to type".

Are there any backwards-compatibility issues you can think of?

I think that "123456" could be a sha1, but I do not see much point in
referencing a sha1 as the argument of "stash show". And it looks like
this code path is called only from is_stash_like(), so presumably the
same logic would apply to other callers.

So I can't immediately think of any downside to this.

> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 92df596..af11cff 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -35,11 +35,12 @@ A stash is by default listed as "WIP on
> 'branchname' ...", but
>  you can give a more descriptive message on the command line when
>  you create one.
>  
> -The latest stash you created is stored in `refs/stash`; older
> -stashes are found in the reflog of this reference and can be named using
> -the usual reflog syntax (e.g. `stash@{0}` is the most recently
> -created stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}`
> -is also possible).
> +The latest stash you created is stored in `refs/stash`; older stashes
> +are found in the reflog of this reference and can be named using the
> +usual reflog syntax (e.g. `stash@{0}` is the most recently created
> +stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}` is also
> +possible). Stashes may also be references by specifying just the stash
> +index (e.g. the integer `n` is equivalent to `stash@{n}`).

Yay, a documentation update. Should it be s/references/referenced/ in
the second-to-last line?

> diff --git a/git-stash.sh b/git-stash.sh
> index 826af18..a0c7f12 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -1,4 +1,4 @@
> -#!/bin/sh
> +#!/bin/bash

I like that you were careful in thinking about bash versus other shells,
but this unfortunately isn't going to work. The #!-line here is just a
placeholder, and is replaced by the contents of $(SHELL_PATH) by the
Makefile.

But that's just the mechanical issue. The greater problem is that we
can't assume that bash is present at all. So we need to figure out a way
to avoid using bash-specific features in the rest of the patch.

> @@ -371,6 +371,14 @@ parse_flags_and_rev()
>   test "$PARSE_CACHE" = "$*" && return 0 # optimisation
>   PARSE_CACHE="$*"
>  
> + args=()
> + for arg
> + do
> + [[ $arg =~ ^[0-9]+$ ]] && arg="stash@{$arg}"
> + args+=("$arg")
> + done
> + set -- "${args[@]}"
> +

So here we replace any pure-numeric arguments with stash@{}. I don't
think this is quite what we want, as it doesn't take into account where
in the parsing we are. When ALLOW_UNKNOWN_FLAGS is set, we might have
arbitrary options to pass to "git diff". So:

  git stash show -l 300 stash@{0}

would mistakenly re-write "300" (actually, I think that is already
broken because we feed the options to rev-parse which is similarly blind
about parsing).

Another one is perhaps:

  git stash show -- 300 ;# only show path "300" of the diff

which rev-parse _does_ handle correctly (although we then complain about
multiple revs, oblivious to the presence of the "--").

So I don't think this is technically a regression in any
currently-functioning behavior, but it seems like a step in the wrong
direction to add yet another layer of blind parsing.

I wonder if we could do better by making our $FLAGS loop a bit smarter,
and stopping at the first non-option (that unfortunately still gets "-l
300" wrong, but is the best we can do, I think). That option becomes
$REV, which we can then munge (as your patch does), and feed to "git
rev-parse --verify" (which it looks like we already do, though it seems
redundant with the earlier call).


> + args=()
> + for arg
> + do
> + [[ $arg =~ ^[0-9]+$ ]] && arg="stash@{$arg}"
> + args+=("$arg")
> + done
> + set -- "${args[@]}"

Obviously all the array-handling is bash-specific, but that goes away if
we just munge the single $REV we find.

The "=~" is another bash-ism; it can be replaced with "expr".

> diff --git a/t/t3907-stash-index.sh b/t/t3907-stash-index.sh
> new file mode 100755
> index 000..72a1838
> --- /dev/null
> +++ b/t/t3907-stash-index.sh

Double yay, tests.

Do we really need a whole new script for this, though? There are already
"stash show" tests in t3903. We should be able to repeat one of them
using "2" instead of "stash@{2}" (for example).

-Peff

PS I think this is your first patch. Welcome to the git list. :)

[PATCH] stash: allow ref of a stash by index

[Aaron M Watson]

Allows stashes to be referenced by index only. Instead of referencing
"stash@{n}" explicitly, it can simply be referenced as "n".

Signed-off-by: Aaron M Watson 
---
 Documentation/git-stash.txt | 11 ---
 git-stash.sh| 10 +-
 t/t3907-stash-index.sh  | 77
+
 3 files changed, 92 insertions(+), 6 deletions(-)
 create mode 100755 t/t3907-stash-index.sh

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 92df596..af11cff 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -35,11 +35,12 @@ A stash is by default listed as "WIP on
'branchname' ...", but
 you can give a more descriptive message on the command line when
 you create one.
 
-The latest stash you created is stored in `refs/stash`; older
-stashes are found in the reflog of this reference and can be named using
-the usual reflog syntax (e.g. `stash@{0}` is the most recently
-created stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}`
-is also possible).
+The latest stash you created is stored in `refs/stash`; older stashes
+are found in the reflog of this reference and can be named using the
+usual reflog syntax (e.g. `stash@{0}` is the most recently created
+stash, `stash@{1}` is the one before it, `stash@{2.hours.ago}` is also
+possible). Stashes may also be references by specifying just the stash
+index (e.g. the integer `n` is equivalent to `stash@{n}`).
 
 OPTIONS
 ---
diff --git a/git-stash.sh b/git-stash.sh
index 826af18..a0c7f12 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -1,4 +1,4 @@
-#!/bin/sh
+#!/bin/bash
 # Copyright (c) 2007, Nanako Shiraishi
 
 dashless=$(basename "$0" | sed -e 's/-/ /')
@@ -371,6 +371,14 @@ parse_flags_and_rev()
    test "$PARSE_CACHE" = "$*" && return 0 # optimisation
    PARSE_CACHE="$*"
 
+   args=()
+   for arg
+   do
+   [[ $arg =~ ^[0-9]+$ ]] && arg="stash@{$arg}"
+   args+=("$arg")
+   done
+   set -- "${args[@]}"
+
    IS_STASH_LIKE=
    IS_STASH_REF=
    INDEX_OPTION=
diff --git a/t/t3907-stash-index.sh b/t/t3907-stash-index.sh
new file mode 100755
index 000..72a1838
--- /dev/null
+++ b/t/t3907-stash-index.sh
@@ -0,0 +1,77 @@
+#!/bin/sh
+#
+# Copyright (c) 2016 Aaron M. Watson
+#
+
+test_description='Test git stash with index ref specification'
+
+. ./test-lib.sh
+
+test_expect_success 'stash some dirty working directory' '
+   echo 1 > file &&
+   git add file &&
+   echo unrelated >other-file &&
+   git add other-file &&
+   test_tick &&
+   git commit -m initial &&
+   echo 2 > file &&
+   git add file &&
+   echo 3 > file &&
+   test_tick &&
+   git stash &&
+   git diff-files --quiet &&
+   git diff-index --cached --quiet HEAD
+'
+
+cat > expect << EOF
+diff --git a/file b/file
+index 0cfbf08..00750ed 100644
+--- a/file
 b/file
+@@ -1 +1 @@
+-2
++3
+EOF
+
+test_expect_success 'applying bogus stash does nothing' '
+   test_must_fail git stash apply 1 &&
+   echo 1 >expect &&
+   test_cmp expect file
+'
+
+test_expect_success 'drop middle stash' '
+   git reset --hard &&
+   echo 8 > file &&
+   git stash &&
+   echo 9 > file &&
+   git stash &&
+   git stash drop 1 &&
+   test 2 = $(git stash list | wc -l) &&
+   git stash apply &&
+   test 9 = $(cat file) &&
+   test 1 = $(git show :file) &&
+   test 1 = $(git show HEAD:file) &&
+   git reset --hard &&
+   git stash drop &&
+   git stash apply &&
+   test 3 = $(cat file) &&
+   test 1 = $(git show :file) &&
+   test 1 = $(git show HEAD:file)
+'
+
+test_expect_success 'invalid ref of the form "n", where n >= N' '
+   git stash clear &&
+   test_must_fail git stash drop 0 &&
+   echo bar5 > file &&
+   echo bar6 > file2 &&
+   git add file2 &&
+   git stash &&
+   test_must_fail git stash drop 1 &&
+   test_must_fail git stash pop 1 &&
+   test_must_fail git stash apply 1 &&
+   test_must_fail git stash show 1 &&
+   test_must_fail git stash branch tmp 1 &&
+   git stash drop
+'
+
+test_done
-- 
2.7.4

Re: Fixup of a fixup not working right

[Philip Oakley]

Hi Robert,
From: "Robert Dailey" 

On Fri, Sep 2, 2016 at 9:22 PM, Junio C Hamano  wrote:

Perhaps a change like this to "rebase -i":

 - The search for "original" when handling "pick fixup! original",
   when it does not find "original", could turn it into "reword
   fixup! original" without changing its position in the instruction
   sequence.



[..]

So this is mostly for my education, since I don't see a difference
from a user-standpoint.


This was a problem in the past.


  Why would "fixup! fixup! original" look for
"original" instead of "fixup! original"? As far as I can see, the
behavior would be the same and the order would be retained since you
are essentially "chaining" the fixups together this way.


The patch that introduced the effect is 22c5b13 (rebase -i: handle fixup! 
fixup! in --autosquash, 2013-06-27). At the time it was possible that the 
commits would be re-ordered based on the full multi-fixup message, and so, 
if you thought you'd corrected a poor fixup, rather correcting the original, 
the conceptual order would change. While the combined diffs still notionally 
add to the same effect, the changes to the context lines may mean they don't 
apply cleanly - keeping them in order makes for a clean application. The 
solution was to ignore multiple fixup!s at the start.




This also
gives you the behavior you want directly because the algorithm will
naturally tie to "fixup! original" even if "original" doesn't exist,
because Git would expect that "fixup! original" will automatically
manage its own order, and that subsequent processed nested "fixup!"
commits would not need to depend on any other commits.


There is a question as to whether the commit you pushed upstream, is classed 
as 'published' and immutable, or still part of the review and modification 
process. At the moment the presumption, in general, is that it would be the 
former, so that you can't fixup the original. I don't see that changing, 
however Junio's suggestion that these extra fixups are 'reworded' so that 
they will rebase (--autosquash) locally to become one commit titled 
something like "fix! original", and then a final rebase that rewords that 
commit back to "fixup! " for pushing upstream to the 'review' process, where 
they can request that it be 'fixed';-) 

Re: Literate programming with git

[Ben North]

Hi Stefan,

Thanks for the remarks.

>> https://github.com/bennorth/git-dendrify
>
> [...]  You get an easy top-level overview what
> the community is interested in via e.g.:
>
> git log --first-parent --oneline
>
> That would be equivalent to showing only
> * Add printing facility
>
> If you run that command on "* Add printing facility"^2
> you would see the headlines of the section.

That's a nice observation on how to use the existing git tools to view a
structured history with different levels of detail.

> However in gits reality we do not have these nice sections
> building on top of each other, as many people are interested in
> different things and build where they see fit.

Yes, reality isn't always clean!  But each individual contributor can
structure their own branch in a hierarchical way if they think it would
be helpful, before publishing it for review.

> How does the linearify/dendrify work with already non-linear history?

If you attempt to 'linearize' a section of history which isn't of the
required hierarchical form, the tool exits without doing anything.
(Because this is only at the experimenting stage, there may well be
situations where it fails to detect an unexpected structure, but see
also next paragraph.)  Similarly, if you attempt to 'dendrify' a section
of history which isn't purely linear, it refuses.

In any case, the tool only ever creates a new branch so your original
history is unaltered.

Thanks,

Ben.

!!!

[admoss1980]

http://www.baidu.com/link?url=DsqnrRyBjH64xj2HvdqZKR4I8iRgR7o9Is6IOc8EiYC#680=ivevjp&4049==97698091

Re: [PATCH 9/9] rebase -i: rearrange fixup/squash lines using the rebase--helper

[Josh Triplett]

On Fri, Sep 02, 2016 at 06:23:42PM +0200, Johannes Schindelin wrote:
> Let's reimplement this with linear complexity (using a hash map to
> match the commits' subject lines) for the common case; Sadly, the
> fixup/squash feature's design neglected performance considerations,
> allowing arbitrary prefixes (read: `fixup! hell` will match the
> commit subject `hello world`), which means that we are stuck with
> quadratic performance in the worst case.

If the performance of that case matters enough, we can do better than
quadratic complexity: maintain a trie of the subjects, allowing prefix
lookups.  (Or hash all the prefixes, which you can do in linear time on
a string: hash next char, save hash, repeat.)  However, that would
pessimize the normal case of either a complete subject or a sha1, due to
the extra time taken constructing the data structure.  Probably not
worth it, if you assume that most "fixup!" subjects come from `git
commit --fixup` or similar automated means.

[PATCH] introduce hex2chr() for converting two hexadecimal digits to a character

[René Scharfe]

Add and use a helper function that decodes the char value of two
hexadecimal digits.  It returns a negative number on error, avoids
running over the end of the given string and doesn't shift negative
values.

Signed-off-by: Rene Scharfe 
---
 cache.h  | 10 ++
 hex.c| 12 ++--
 pkt-line.c   | 23 ++-
 pretty.c | 13 +
 ref-filter.c | 20 +---
 url.c| 21 +
 6 files changed, 21 insertions(+), 78 deletions(-)

diff --git a/cache.h b/cache.h
index b780a91..b0dae4b 100644
--- a/cache.h
+++ b/cache.h
@@ -1139,6 +1139,16 @@ static inline unsigned int hexval(unsigned char c)
return hexval_table[c];
 }
 
+/*
+ * Convert two consecutive hexadecimal digits into a char.  Return a
+ * negative value on error.  Don't run over the end of short strings.
+ */
+static inline int hex2chr(const char *s)
+{
+   int val = hexval(s[0]);
+   return (val < 0) ? val : (val << 4) | hexval(s[1]);
+}
+
 /* Convert to/from hex/sha1 representation */
 #define MINIMUM_ABBREV minimum_abbrev
 #define DEFAULT_ABBREV default_abbrev
diff --git a/hex.c b/hex.c
index 9619b67..ab2610e 100644
--- a/hex.c
+++ b/hex.c
@@ -39,16 +39,8 @@ int get_sha1_hex(const char *hex, unsigned char *sha1)
 {
int i;
for (i = 0; i < GIT_SHA1_RAWSZ; i++) {
-   unsigned int val;
-   /*
-* hex[1]=='\0' is caught when val is checked below,
-* but if hex[0] is NUL we have to avoid reading
-* past the end of the string:
-*/
-   if (!hex[0])
-   return -1;
-   val = (hexval(hex[0]) << 4) | hexval(hex[1]);
-   if (val & ~0xff)
+   int val = hex2chr(hex);
+   if (val < 0)
return -1;
*sha1++ = val;
hex += 2;
diff --git a/pkt-line.c b/pkt-line.c
index 62fdb37..30489c6 100644
--- a/pkt-line.c
+++ b/pkt-line.c
@@ -172,27 +172,8 @@ static int get_packet_data(int fd, char **src_buf, size_t 
*src_size,
 
 static int packet_length(const char *linelen)
 {
-   int n;
-   int len = 0;
-
-   for (n = 0; n < 4; n++) {
-   unsigned char c = linelen[n];
-   len <<= 4;
-   if (c >= '0' && c <= '9') {
-   len += c - '0';
-   continue;
-   }
-   if (c >= 'a' && c <= 'f') {
-   len += c - 'a' + 10;
-   continue;
-   }
-   if (c >= 'A' && c <= 'F') {
-   len += c - 'A' + 10;
-   continue;
-   }
-   return -1;
-   }
-   return len;
+   int val = hex2chr(linelen);
+   return (val < 0) ? val : (val << 8) | hex2chr(linelen + 2);
 }
 
 int packet_read(int fd, char **src_buf, size_t *src_len,
diff --git a/pretty.c b/pretty.c
index 9609afb..9788bd8 100644
--- a/pretty.c
+++ b/pretty.c
@@ -1065,7 +1065,7 @@ static size_t format_commit_one(struct strbuf *sb, /* in 
UTF-8 */
const struct commit *commit = c->commit;
const char *msg = c->message;
struct commit_list *p;
-   int h1, h2;
+   int ch;
 
/* these are independent of the commit */
switch (placeholder[0]) {
@@ -1089,14 +1089,11 @@ static size_t format_commit_one(struct strbuf *sb, /* 
in UTF-8 */
return 1;
case 'x':
/* %x00 == NUL, %x0a == LF, etc. */
-   if (0 <= (h1 = hexval_table[0xff & placeholder[1]]) &&
-   h1 <= 16 &&
-   0 <= (h2 = hexval_table[0xff & placeholder[2]]) &&
-   h2 <= 16) {
-   strbuf_addch(sb, (h1<<4)|h2);
-   return 3;
-   } else
+   ch = hex2chr(placeholder + 1);
+   if (ch < 0)
return 0;
+   strbuf_addch(sb, ch);
+   return 3;
case 'w':
if (placeholder[1] == '(') {
unsigned long width = 0, indent1 = 0, indent2 = 0;
diff --git a/ref-filter.c b/ref-filter.c
index bc551a7..9adbb8a 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1576,24 +1576,6 @@ void ref_array_sort(struct ref_sorting *sorting, struct 
ref_array *array)
qsort(array->items, array->nr, sizeof(struct ref_array_item *), 
compare_refs);
 }
 
-static int hex1(char ch)
-{
-   if ('0' <= ch && ch <= '9')
-   return ch - '0';
-   else if ('a' <= ch && ch <= 'f')
-   return ch - 'a' + 10;
-   else if ('A' <= ch && ch <= 'F')
-   return ch - 'A' + 10;
-   return -1;
-}
-static int hex2(const char *cp)
-{
-   if (cp[0] && cp[1])
-   return (hex1(cp[0]) << 4) | hex1(cp[1]);
-   else
-   return -1;
-}
-
 static void append_literal(const char *cp, 

[PATCH] compat: move strdup(3) replacement to its own file

[René Scharfe]

Move our implementation of strdup(3) out of compat/nedmalloc/ and allow
it to be used independently from USE_NED_ALLOCATOR.  This reduces the
difference of our copy of nedmalloc from the original, making it easier
to update, and allows for easier testing and reusing of our version of
strdup().

Signed-off-by: Rene Scharfe 
---
 Makefile | 17 ++---
 compat/nedmalloc/nedmalloc.c | 16 
 compat/strdup.c  | 11 +++
 git-compat-util.h|  8 
 4 files changed, 33 insertions(+), 19 deletions(-)
 create mode 100644 compat/strdup.c

diff --git a/Makefile b/Makefile
index d96ecb7..7f18492 100644
--- a/Makefile
+++ b/Makefile
@@ -296,6 +296,11 @@ all::
 # Define USE_NED_ALLOCATOR if you want to replace the platforms default
 # memory allocators with the nedmalloc allocator written by Niall Douglas.
 #
+# Define OVERRIDE_STRDUP to override the libc version of strdup(3).
+# This is necessary when using a custom allocator in order to avoid
+# crashes due to allocation and free working on different 'heaps'.
+# It's defined automatically if USE_NED_ALLOCATOR is set.
+#
 # Define NO_REGEX if you have no or inferior regex support in your C library.
 #
 # Define HAVE_DEV_TTY if your system can open /dev/tty to interact with the
@@ -1456,8 +1461,14 @@ ifdef NATIVE_CRLF
 endif
 
 ifdef USE_NED_ALLOCATOR
-   COMPAT_CFLAGS += -Icompat/nedmalloc
-   COMPAT_OBJS += compat/nedmalloc/nedmalloc.o
+   COMPAT_CFLAGS += -Icompat/nedmalloc
+   COMPAT_OBJS += compat/nedmalloc/nedmalloc.o
+   OVERRIDE_STRDUP = YesPlease
+endif
+
+ifdef OVERRIDE_STRDUP
+   COMPAT_CFLAGS += -DOVERRIDE_STRDUP
+   COMPAT_OBJS += compat/strdup.o
 endif
 
 ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
@@ -2029,7 +2040,7 @@ endif
 
 ifdef USE_NED_ALLOCATOR
 compat/nedmalloc/nedmalloc.sp compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \
-   -DNDEBUG -DOVERRIDE_STRDUP -DREPLACE_SYSTEM_ALLOCATOR
+   -DNDEBUG -DREPLACE_SYSTEM_ALLOCATOR
 compat/nedmalloc/nedmalloc.sp: SPARSE_FLAGS += -Wno-non-pointer-null
 endif
 
diff --git a/compat/nedmalloc/nedmalloc.c b/compat/nedmalloc/nedmalloc.c
index 2d4ef59..1cc31c3 100644
--- a/compat/nedmalloc/nedmalloc.c
+++ b/compat/nedmalloc/nedmalloc.c
@@ -948,22 +948,6 @@ void **nedpindependent_comalloc(nedpool *p, size_t elems, 
size_t *sizes, void **
return ret;
 }
 
-#ifdef OVERRIDE_STRDUP
-/*
- * This implementation is purely there to override the libc version, to
- * avoid a crash due to allocation and free on different 'heaps'.
- */
-char *strdup(const char *s1)
-{
-   size_t len = strlen(s1) + 1;
-   char *s2 = malloc(len);
-
-   if (s2)
-   memcpy(s2, s1, len);
-   return s2;
-}
-#endif
-
 #if defined(__cplusplus)
 }
 #endif
diff --git a/compat/strdup.c b/compat/strdup.c
new file mode 100644
index 000..f3fb978
--- /dev/null
+++ b/compat/strdup.c
@@ -0,0 +1,11 @@
+#include "../git-compat-util.h"
+
+char *gitstrdup(const char *s1)
+{
+   size_t len = strlen(s1) + 1;
+   char *s2 = malloc(len);
+
+   if (s2)
+   memcpy(s2, s1, len);
+   return s2;
+}
diff --git a/git-compat-util.h b/git-compat-util.h
index db89ba7..93f6f23 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -663,6 +663,14 @@ void *gitmemmem(const void *haystack, size_t haystacklen,
 const void *needle, size_t needlelen);
 #endif
 
+#ifdef OVERRIDE_STRDUP
+#ifdef strdup
+#undef strdup
+#endif
+#define strdup gitstrdup
+char *gitstrdup(const char *s);
+#endif
+
 #ifdef NO_GETPAGESIZE
 #define getpagesize() sysconf(_SC_PAGESIZE)
 #endif
-- 
2.10.0

[PATCH 5/6] git-gui: Update Japanese translation

[Satoshi Yasushima]

Signed-off-by: Satoshi Yasushima 
---
 po/ja.po | 77 +---
 1 file changed, 35 insertions(+), 42 deletions(-)

diff --git a/po/ja.po b/po/ja.po
index 23974cc..deaf8e3 100644
--- a/po/ja.po
+++ b/po/ja.po
@@ -102,6 +102,8 @@ msgstr "準備完了"
 msgid ""
 "Display limit (gui.maxfilesdisplayed = %s) reached, not showing all %s files."
 msgstr ""
+"表示可能な限界 (gui.maxfilesdisplayed = %s) に達しため、全体で%s個のファイル"
+"を表示できません"
 
 #: git-gui.sh:2101
 msgid "Unmodified"
@@ -128,23 +130,20 @@ msgid "File type changed, not staged"
 msgstr "ファイル型変更、コミット未予定"
 
 #: git-gui.sh:2109 git-gui.sh:2110
-#, fuzzy
 msgid "File type changed, old type staged for commit"
-msgstr "ファイル型変更、コミット未予定"
+msgstr "ファイル型変更、旧型コミット予定済"
 
 #: git-gui.sh:2111
 msgid "File type changed, staged"
 msgstr "ファイル型変更、コミット予定済"
 
 #: git-gui.sh:2112
-#, fuzzy
 msgid "File type change staged, modification not staged"
-msgstr "ファイル型変更、コミット未予定"
+msgstr "ファイル型変更コミット予定済、変更コミット未予定"
 
 #: git-gui.sh:2113
-#, fuzzy
 msgid "File type change staged, file missing"
-msgstr "ファイル型変更、コミット予定済"
+msgstr "ファイル型変更コミット予定済、ファイル無し"
 
 #: git-gui.sh:2115
 msgid "Untracked, not staged"
@@ -408,10 +407,9 @@ msgstr "SSH キーを表示"
 
 #: git-gui.sh:3014 git-gui.sh:3146
 msgid "Usage"
-msgstr ""
+msgstr "使い方"
 
 #: git-gui.sh:3095 lib/blame.tcl:573
-#, fuzzy
 msgid "Error"
 msgstr "エラー"
 
@@ -1112,9 +1110,8 @@ msgid "Find Text..."
 msgstr "テキストを検索"
 
 #: lib/blame.tcl:288
-#, fuzzy
 msgid "Goto Line..."
-msgstr "複製…"
+msgstr "指定行に移動…"
 
 #: lib/blame.tcl:297
 msgid "Do Full Copy Detection"
@@ -1310,7 +1307,7 @@ msgstr "共有(最高速・非推奨・バックアップ無し)"
 
 #: lib/choose_repository.tcl:545
 msgid "Recursively clone submodules too"
-msgstr ""
+msgstr "サブモジュールも再帰的に複製する"
 
 #: lib/choose_repository.tcl:579 lib/choose_repository.tcl:626
 #: lib/choose_repository.tcl:772 lib/choose_repository.tcl:842
@@ -1435,12 +1432,11 @@ msgstr "ファイル"
 
 #: lib/choose_repository.tcl:981
 msgid "Cannot clone submodules."
-msgstr ""
+msgstr "サブモジュールが複製できません。"
 
 #: lib/choose_repository.tcl:990
-#, fuzzy
 msgid "Cloning submodules"
-msgstr "%s から複製しています"
+msgstr "サブモジュールを複製しています"
 
 #: lib/choose_repository.tcl:1015
 msgid "Initial file checkout failed."
@@ -1515,11 +1511,11 @@ msgstr "前"
 
 #: lib/search.tcl:52
 msgid "RegExp"
-msgstr ""
+msgstr "正規表現"
 
 #: lib/search.tcl:54
 msgid "Case"
-msgstr ""
+msgstr "大文字小文字を区別"
 
 #: lib/status_bar.tcl:87
 #, tcl-format
@@ -1635,9 +1631,9 @@ msgid "Running %s requires a selected file."
 msgstr "ファイルを選択してから %s を起動してください。"
 
 #: lib/tools.tcl:91
-#, fuzzy, tcl-format
+#, tcl-format
 msgid "Are you sure you want to run %1$s on file \"%2$s\"?"
-msgstr "本当に %s を起動しますか?"
+msgstr "本当にファイル \"%2$s\"で %1$s を起動しますか?"
 
 #: lib/tools.tcl:95
 #, tcl-format
@@ -1817,16 +1813,15 @@ msgstr "トラッキングブランチを合わせる"
 
 #: lib/option.tcl:151
 msgid "Use Textconv For Diffs and Blames"
-msgstr ""
+msgstr "diff と注釈に textconv を使う"
 
 #: lib/option.tcl:152
 msgid "Blame Copy Only On Changed Files"
 msgstr "変更されたファイルのみコピー検知を行なう"
 
 #: lib/option.tcl:153
-#, fuzzy
 msgid "Maximum Length of Recent Repositories List"
-msgstr "最近使ったリポジトリ"
+msgstr "最近使ったリポジトリ一覧の上限"
 
 #: lib/option.tcl:154
 msgid "Minimum Letters To Blame Copy On"
@@ -1842,7 +1837,7 @@ msgstr "diff の文脈行数"
 
 #: lib/option.tcl:157
 msgid "Additional Diff Parameters"
-msgstr ""
+msgstr "diff の追加引数"
 
 #: lib/option.tcl:158
 msgid "Commit Message Text Width"
@@ -1858,19 +1853,19 @@ msgstr "ファイル内容のデフォールトエンコーディング"
 
 #: lib/option.tcl:161
 msgid "Warn before committing to a detached head"
-msgstr ""
+msgstr "分離 HEAD のコミット前に警告する"
 
 #: lib/option.tcl:162
 msgid "Staging of untracked files"
-msgstr ""
+msgstr "管理外のファイルをコミット予定する"
 
 #: lib/option.tcl:163
 msgid "Show untracked files"
-msgstr ""
+msgstr "管理外のファイルを表示する"
 
 #: lib/option.tcl:164
 msgid "Tab spacing"
-msgstr ""
+msgstr "タブ幅"
 
 #: lib/option.tcl:210
 msgid "Change"
@@ -1979,22 +1974,19 @@ msgstr "%s から削除されたトラッキング・ブランチを刈ってい
 
 #: lib/transport.tcl:25
 msgid "fetch all remotes"
-msgstr ""
+msgstr "すべてのリモートを取得"
 
 #: lib/transport.tcl:26
-#, fuzzy
 msgid "Fetching new changes from all remotes"
-msgstr "%s から新しい変更をフェッチしています"
+msgstr "すべてのリモートから新しい変更をフェッチしています"
 
 #: lib/transport.tcl:40
-#, fuzzy
 msgid "remote prune all remotes"
-msgstr "リモート刈込 %s"
+msgstr "リモート刈込 すべてのリモート"
 
 #: lib/transport.tcl:41
-#, fuzzy
 msgid "Pruning tracking branches deleted from all remotes"
-msgstr "%s から削除されたトラッキング・ブランチを刈っています"
+msgstr "すべてのリモートから削除されたトラッキング・ブランチを刈っています"
 
 #: lib/transport.tcl:54 lib/transport.tcl:92 lib/transport.tcl:110
 #: lib/remote_add.tcl:162
@@ -2247,7 +2239,7 @@ msgstr "コミットに %s を加えています"
 #: lib/index.tcl:380
 #, tcl-format
 msgid "Stage %d untracked files?"
-msgstr ""
+msgstr "管理外の %d ファイルをコミット予定としますか？"
 
 #: lib/index.tcl:428
 #, tcl-format
@@ -2452,6 +2444,13 @@ msgid ""
 " \n"
 " Do you really want to proceed with your Commit?"
 msgstr ""
+"分離 HEAD での変更をコミットしようとしています。"
+"これは潜在的に危険な行為で、理由は別のブランチへの切り替えで"

[PATCH 6/6] git-gui: Update Japanese information

[Satoshi Yasushima]

Signed-off-by: Satoshi Yasushima 
---
 po/ja.po | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/po/ja.po b/po/ja.po
index deaf8e3..208651c 100644
--- a/po/ja.po
+++ b/po/ja.po
@@ -1,15 +1,17 @@
 # Translation of git-gui to Japanese
 # Copyright (C) 2007 Shawn Pearce
 # This file is distributed under the same license as the git-gui package.
+#
 # しらいし ななこ , 2007.
+# Satoshi Yasushima , 2016.
 #
 msgid ""
 msgstr ""
 "Project-Id-Version: git-gui\n"
 "Report-Msgid-Bugs-To: \n"
 "POT-Creation-Date: 2016-05-27 17:52+0900\n"
-"PO-Revision-Date: 2010-02-02 19:03+0900\n"
-"Last-Translator: しらいし ななこ \n"
+"PO-Revision-Date: 2016-06-22 12:50+0900\n"
+"Last-Translator: Satoshi Yasushima \n"
 "Language-Team: Japanese\n"
 "Language: ja\n"
 "MIME-Version: 1.0\n"
-- 
2.8.2.windows.1

[PATCH 4/6] git-gui: Add Japanese language code

[Satoshi Yasushima]

Signed-off-by: Satoshi Yasushima 
---
 po/ja.po | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/po/ja.po b/po/ja.po
index b140e8b..23974cc 100644
--- a/po/ja.po
+++ b/po/ja.po
@@ -11,7 +11,7 @@ msgstr ""
 "PO-Revision-Date: 2010-02-02 19:03+0900\n"
 "Last-Translator: しらいし ななこ \n"
 "Language-Team: Japanese\n"
-"Language: \n"
+"Language: ja\n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
-- 
2.8.2.windows.1

[PATCH 2/6] git-gui: The term unified for blame in Japanese

[Satoshi Yasushima]

Signed-off-by: Satoshi Yasushima 
---
 po/ja.po | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/po/ja.po b/po/ja.po
index 8a2c16f..b692b5c 100644
--- a/po/ja.po
+++ b/po/ja.po
@@ -598,7 +598,7 @@ msgstr "文脈を見せる"
 
 #: lib/blame.tcl:291
 msgid "Blame Parent Commit"
-msgstr "親コミットを註釈"
+msgstr "親コミットを注釈"
 
 #: lib/blame.tcl:450
 #, tcl-format
@@ -2052,7 +2052,7 @@ msgstr "コピーを検知する最少文字数"
 
 #: lib/option.tcl:151
 msgid "Blame History Context Radius (days)"
-msgstr "註釈する履歴半径（日数）"
+msgstr "注釈する履歴半径（日数）"
 
 #: lib/option.tcl:152
 msgid "Number of Diff Context Lines"
-- 
2.8.2.windows.1

[PATCH 1/6] git-gui: The term unified for remote in Japanese

[Satoshi Yasushima]

Signed-off-by: Satoshi Yasushima 
---
 po/ja.po | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/po/ja.po b/po/ja.po
index 9aff249..8a2c16f 100644
--- a/po/ja.po
+++ b/po/ja.po
@@ -765,7 +765,8 @@ msgstr "トラッキング・ブランチを選択して下さい。"
 #: lib/branch_create.tcl:140
 #, tcl-format
 msgid "Tracking branch %s is not a branch in the remote repository."
-msgstr "トラッキング・ブランチ %s は遠隔リポジトリのブランチではありません。"
+msgstr ""
+"トラッキング・ブランチ %s はリモートリポジトリのブランチではありません。"
 
 #: lib/branch_create.tcl:153 lib/branch_rename.tcl:86
 msgid "Please supply a branch name."
@@ -2192,7 +2193,7 @@ msgstr "%2$s にある %1$s をセットアップします"
 
 #: lib/remote_branch_delete.tcl:29 lib/remote_branch_delete.tcl:34
 msgid "Delete Branch Remotely"
-msgstr "遠隔でブランチ削除"
+msgstr "リモートブランチ削除"
 
 #: lib/remote_branch_delete.tcl:47
 msgid "From Repository"
@@ -2504,7 +2505,7 @@ msgstr "%s から新しい変更をフェッチしています"
 #: lib/transport.tcl:18
 #, tcl-format
 msgid "remote prune %s"
-msgstr "遠隔刈込 %s"
+msgstr "リモート刈込 %s"
 
 #: lib/transport.tcl:19
 #, tcl-format
-- 
2.8.2.windows.1

Re: Fixup of a fixup not working right

[Robert Dailey]

On Fri, Sep 2, 2016 at 9:22 PM, Junio C Hamano  wrote:
> Perhaps a change like this to "rebase -i":
>
>  - The search for "original" when handling "pick fixup! original",
>when it does not find "original", could turn it into "reword
>fixup! original" without changing its position in the instruction
>sequence.

If it did this, then later when I'm ready to merge and all
contributions upstream are completed, I would most likely lose track
of which original commit to fixup to. This would only be an issue if
you gave the option to reword the "fixup! original". Rewording it, to
me, inherently means you no longer want to meld it into another commit
and instead intend on it being independent (looking at this
semantically, without relying on translating the commit message
itself, even though it may clearly tell me to squash it to something
else, that's not as reliable as the fixup! mechanics IMHO).

>  - The search for "original" when handling "pick fixup! fixup!
>original", could be (probably unconditionally) changed to look
>for "fixup! original" to amend, instead of looking for "original"
>as the current code (this is your "separate issue").  The same
>"if the commit to be amended is not found, turn it into reword"
>rule from the above applies to this one, too.

So this is mostly for my education, since I don't see a difference
from a user-standpoint. Why would "fixup! fixup! original" look for
"original" instead of "fixup! original"? As far as I can see, the
behavior would be the same and the order would be retained since you
are essentially "chaining" the fixups together this way. This also
gives you the behavior you want directly because the algorithm will
naturally tie to "fixup! original" even if "original" doesn't exist,
because Git would expect that "fixup! original" will automatically
manage its own order, and that subsequent processed nested "fixup!"
commits would not need to depend on any other commits.

[ANNOUNCE] Git for Windows 2.10.0

[Johannes Schindelin]

Dear Git users,

It is my pleasure to announce that Git for Windows 2.10.0 is available.
This time, I even blogged about it, primarily because I am so excited
about the speed improvements of rebase -i:

https://blogs.msdn.microsoft.com/visualstudioalm/2016/09/03/whats-new-in-git-for-windows-2-10/

As always, you can download it from: https://git-for-windows.github.io/

Changes since Git for Windows v2.9.3(2) (August 25th 2016)

New Features

  • Comes with Git v2.10.0.
  • The git rebase -i command was made faster by reimplementing large
parts in C.
  • After helping the end-users to use the new defaults for PATH and
FSCache, the installer now respects the saved settings again.
  • git version --build-options now also reports the architecture.

Bug Fixes

  • When upgrading Git for Windows, the installer no longer opens a
second window while uninstalling the previous version.
  • Git for Windows' SDK can build an installer out of the box again,
without requiring an extra package to be installed.

Filename | SHA-256
 | ---
Git-2.10.0-64-bit.exe | 
d624e08991a7a28b7156e384503228273e2da4c74c20b21152a97f4a304886da
Git-2.10.0-32-bit.exe | 
5d565da086a0b7aca46face8439146826ddd51ffeb286845be89fe4c2713d63c
PortableGit-2.10.0-64-bit.7z.exe | 
d23b81e88949042d34e97bfd1e5b579d4b6aadec085c6ff6b05c4579da1d49e4
PortableGit-2.10.0-32-bit.7z.exe | 
89940cca2a8e1b18b5ed6e3d46c97ea4fcfe1628cda3ae452cd2a8984a3c25c8
MinGit-2.10.0-64-bit.zip | 
2e1101ec57da526728704c04792293613f3c5aa18e65f13a4129d00b54de2087
MinGit-2.10.0-32-bit.zip | 
36f890870126dcf840d87eaec7e55b8a483bc336ebf8970de2f9d549a3cfc195
Git-2.10.0-64-bit.tar.bz2 | 
4c98383ed38ba6000cad9d80c1819f686692d90e004207726085e823f6928ebc
Git-2.10.0-32-bit.tar.bz2 | 
0b132212b858743934d47f40f38e29eef0c1a06c735b0e9889ab6e0d9c195d81

Ciao,
Johannes

Re: [RFC/PATCH 2/2] WIP xdiff: markup duplicates differently

[Jakub Narębski]

W dniu 03.09.2016 o 05:31, Stefan Beller pisze:

> When moving code (e.g. a function is moved to another part of the file or
> to a different file), the review process is different than reviewing new
> code. When reviewing moved code we are only interested in the diff as
> where there are differences in the moved code, e.g. namespace changes.
> 
> However the inner part of these moved texts should not change.
> To aid a developer reviewing such code, emit it with a different prefix
> than the usual +,- to indicate it is overlapping code.

What would be this different prefix?


Side note: I wonder if the cousin of unified diff, namely context diff[1],
is something that we can and should support.

[1]: https://www.gnu.org/software/diffutils/manual/html_node/Context-Format.html
 
https://www.gnu.org/software/diffutils/manual/html_node/Detailed-Context.html

*** lao 2002-02-21 23:30:39.942229878 -0800
--- tzu 2002-02-21 23:30:50.442260588 -0800
***
*** 1,7 
- The Way that can be told of is not the eternal Way;
- The name that can be named is not the eternal name.
  The Nameless is the origin of Heaven and Earth;
! The Named is the mother of all things.
  Therefore let there always be non-being,
so we may see their subtlety,
  And let there always be being,
--- 1,6 
  The Nameless is the origin of Heaven and Earth;
! The named is the mother of all things.
! 
  Therefore let there always be non-being,
so we may see their subtlety,
  And let there always be being,
***
*** 9,11 
--- 8,13 
  The two are the same,
  But after they are produced,
they have different names.
+ They both may be called deep and profound.
+ Deeper and more profound,
+ The door of all subtleties!

Re: A note from the maintainer

[Jakub Narębski]

W dniu 03.09.2016 o 04:17, Junio C Hamano pisze:

> Please remember to always state
> 
>  - what you wanted to achieve;
> 
>  - what you did (the version of git and the command sequence to reproduce
>the behavior);

I wonder if it be worth adding to not use aliases (or expand them).  I have
seen quite a few such questions on StackOverflow...

> 
>  - what you saw happen (X above);
> 
>  - what you expected to see (Y above); and
> 
>  - how the last two are different.

Re: [PATCH 4/9] rebase -i: also expand/collapse the SHA-1s via the rebase--helper

[Johannes Schindelin]

Hi Dennis,

On Fri, 2 Sep 2016, Dennis Kaarsemaker wrote:

> On vr, 2016-09-02 at 18:23 +0200, Johannes Schindelin wrote:
> > This is crucial to improve performance on Windows, as the speed is now
> > mostly dominated by the SHA-1 transformation (because it spawns a new
> > rev-parse process for *every* line, and spawning processes is pretty
> > slow from Git for Windows' MSYS2 Bash).
> 
> I see these functions only used as part of an shorten-edit-expand
> sequence. Why not do a git rebase-helper --edit-todo instead? Saves
> another few process spawnings.

It would make sense to consolidate the steps, yes. I just tried to be
careful in my incremental approach and am fairly certain about the current
revision faithfully replicating the previous behavior.

> Something for yet another later followup patch?

Sure. Probably more than one patch, though: I could imagine that a minor
refactoring would allow us to read in the todo script once, then apply the
individual processing steps in-memory, and then write out the new todo
script.

And then we can implement an --edit-todo with an optional --initial flag
that triggers the check for validity and the rearranging of the
fixup/squash commands (when the user calls `git rebase --edit-todo`,
neither of those steps should be run).

Maybe you will want to have a look into that while I am mostly offline?

Ciao,
Dscho

Re: [RFC/PATCH 0/2] Color moved code differently

[Junio C Hamano]

Stefan Beller  writes:

> A line is colored differently if that line and the surroundign 2 lines
> appear as-is in the opposite part of the diff.
>
> Example:
> http://i.imgur.com/ay84q0q.png
>
> Or apply these patches and 
> git show e28eae3184b26d3cf3293e69403babb5c575342c
> git show bc9204d4ef6e0672389fdfb0d398fa9a39dba3d5
> git show 8465541e8ce8eaf16e66ab847086779768c18f2d

I like this as a concept.  Two quick comments are

 * On 1/2, you would also need to teach diff-color-slot the
   correspondence between the name used by configuration and the
   enum used as the index into the diff_colors[] array.  I think
   these are not "DUPLICATE", but "MOVE", so I'd suggest renaming
   dup-new and dup-old to some words or phrases that have "MOVED"
   and "TO" or "FROM" in them (e.g. "DIFF_MOVED_FROM",
   "DIFF_MOVED_TO").

 * On 2/2, doing it at xdiff.c level may be limiting this good idea
   to flourish to its full potential, as the interface is fed only
   one diff_filepair at a time.  All the examples you pointed at
   above have line movement within a single path because of this
   design limitation.  I do not think 2/2 would serve as a small but
   good first step to build on top of to enhance the feature to
   notice line movements across files and the design (not the
   implementation) needs rethinking.

The idea has a potential to help reviewing inter-file movement of
lines in 3b0c4200 ("apply: move libified code from builtin/apply.c
to apply.{c,h}", 2016-08-08).  You can see what was _changed_ in the
part that has been moved across files with "show -B -M", and
sometimes that is useful, but at the same time, you cannot see what
was moved without changing, which often is necessary to understand
the changes and notice things like "you moved this across files
without changing, but this and that you did not change need to be
adjusted".

The coloring of "these are moved verbatim" in the style this series
gives would be very helpful for reviewing such a change.

Re: [PATCH 01/34] sequencer: support a new action: 'interactive rebase'

[Johannes Schindelin]

Hi Kevin,

On Fri, 2 Sep 2016, Kevin Daudt wrote:

> On Wed, Aug 31, 2016 at 10:54:02AM +0200, Johannes Schindelin wrote:
> > @@ -43,16 +51,20 @@ static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, 
> > "rebase-merge/gpg_sign_opt")
> >  /* We will introduce the 'interactive rebase' mode later */
> >  static inline int is_rebase_i(const struct replay_opts *opts)
> >  {
> > -   return 0;
> > +   return opts->action == REPLAY_INTERACTIVE_REBASE;
> >  }
> >  
> >  static const char *get_dir(const struct replay_opts *opts)
> >  {
> > +   if (is_rebase_i(opts))
> > +   return rebase_path();
> > return git_path_seq_dir();
> >  }
> >  
> >  static const char *get_todo_path(const struct replay_opts *opts)
> >  {
> > +   if (is_rebase_i(opts))
> > +   return rebase_path_todo();
> > return git_path_todo_file();
> >  }
> 
> This patch fails to apply for me because function is_rebase_i has never
> been introduced before (no record of it anywhere). Currently, only
> IS_REBASE_I macro is present.

I did not send out a new iteration of the prepare-sequencer patch series
(mostly because I wanted reviewers to look at the later patch series,
rather than re-review a 2nd iteration of something they already saw).

But I did address the concerns mentioned in the review already, of course,
because part of the reason to show those patch series was to get valuable
feedback before including the work in Git for Windows v2.10.0.

You can find the current iteration of the prepare-sequencer here:
https://github.com/dscho/git/compare/libify-sequencer...prepare-sequencer

Please note that I will most likely try to address some l10n concerns
before sending out the next iteration of the patch series.

The patch introducing is_rebase_i() (and no longer IS_REBASE_I()) is:
https://github.com/dscho/git/commit/76d272020bb72618957308f06083c807efe59aca

If you want to have the latest iteration of the entire patch thicket, just
`git fetch https://github.com/dscho/git interactive-rebase`. I update that
more frequently than I send out updates via mail.

Ciao,
Johannes