[PATCH 4/8] diff.c: reorder arguments for emit_line_ws_markup

2018-08-13 Thread Stefan Beller
The order shall be all colors first, then the content, flags at the end. The colors are in the order of occurrence, i.e. first the color for the sign and then the color for the rest of the line. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- diff.c | 12 ++-- 1 file

[PATCH 3/8] diff.c: simplify caller of emit_line_0

2018-08-13 Thread Stefan Beller
Due to the previous condition we know "set_sign != NULL" at that point. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- diff.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/diff.c b/diff.c index ae131495216..f6df18af913 100644 --- a/diff.c +++ b/diff.c @@

[PATCH 7/8] diff.c: omit check for line prefix in emit_line_0

2018-08-13 Thread Stefan Beller
As the previous patch made sure we only call emit_line_0 once per line, we do not need the work around introduced in f7c3b4e2d8c (diff: add an internal option to dual-color diffs of diffs, 2018-08-13) that would ensure we'd emit 'diff_line_prefix(o)' just once per line. By having just one call of

[PATCH 1/8] test_decode_color: understand FAINT and ITALIC

2018-08-13 Thread Stefan Beller
Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- t/test-lib-functions.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 2b2181dca09..be8244c47b5 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@

[PATCH 8/8] diff.c: rewrite emit_line_0 more understandably

2018-08-13 Thread Stefan Beller
Rewrite emit_line_0 to have fewer (nested) conditions. The change in 'emit_line' makes sure that 'first' is never user data, but always under our control, a sign or special character in the beginning of the line (or 0, in which case we ignore it). So from now on, let's pass only a diff marker or

[PATCH 6/8] diff: use emit_line_0 once per line

2018-08-13 Thread Stefan Beller
All lines that use emit_line_0 multiple times per line, are combined into a single call to emit_line_0, making use of the 'set' argument. We gain a little efficiency here, as we can omit emission of color and accompanying reset if 'len == 0'. Signed-off-by: Stefan Beller Signed-off-by: Junio C

Re: wishlist: "--cached|--staged" to "git commit file(s)"

2018-08-13 Thread Junio C Hamano
Yaroslav Halchenko writes: > command. ATM there is no non-interactive (via --patch/--interactive I > think it is possible) way to commit selected subset of staged files not > from the worktree (as it is done with "git commit file(s)") but from the > index. Hmph, so edit A B C

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-13 Thread Jeff King
On Mon, Aug 13, 2018 at 07:15:23PM +0200, René Scharfe wrote: > Am 11.08.2018 um 22:59 schrieb René Scharfe: > > If the current oidset implementation is so bad, why not replace it with > > one based on oid_array? ;-) > > > > Intuitively I'd try a hashmap with no payload and open addressing via >

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-13 Thread Jeff King
On Mon, Aug 13, 2018 at 07:15:19PM +0200, René Scharfe wrote: > Getting sidetracked here, but the following patch helps both sides a bit: > > -- >8 -- > Subject: [PATCH] cat-file: reuse strbuf in batch_object_write() > > Avoid allocating and then releasing memory for constructing the output >

Re: [PATCH v4 2/5] unpack-trees: add performance tracing

2018-08-13 Thread Junio C Hamano
Jeff King writes: > I can buy the argument that it's nice to have some form of profiling > that works everywhere, even if it's lowest-common-denominator. I just > wonder if we could be investing effort into tooling around existing > solutions that will end up more powerful and flexible in the

[PATCH 5/7] builtin/submodule--helper: factor out method to update a single submodule

2018-08-13 Thread Stefan Beller
In a later patch we'll find this method handy. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index

[PATCH 4/7] builtin/submodule--helper: store update_clone information in a struct

2018-08-13 Thread Stefan Beller
The information that is printed for update_submodules in 'submodule--helper update-clone' and consumed by 'git submodule update' is stored as a string per submodule. This made sense at the time of 48308681b07 (git submodule update: have a dedicated helper for cloning, 2016-02-29), but as we want

[PATCH 2/7] git-submodule.sh: rename unused variables

2018-08-13 Thread Stefan Beller
The 'mode' variable is not used in cmd_update for its original purpose, rename it to 'dummy' as it only serves the purpose to abort quickly documenting this knowledge. The variable 'stage' is also not used any more in cmd_update, so remove it. This went unnoticed as first each function used the

[PATCH 3/7] builtin/submodule--helper: factor out submodule updating

2018-08-13 Thread Stefan Beller
Separate the command line parsing from the actual execution of the command within the repository. For now there is not a lot of execution as most of it is still in git-submodule.sh. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- builtin/submodule--helper.c | 59

[PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree

2018-08-13 Thread Stefan Beller
e98317508c0 (submodule: ensure core.worktree is set after update, 2018-06-18) was overly aggressive in calling connect_work_tree_and_git_dir as that ensures both the 'core.worktree' configuration is set as well as setting up correct gitlink file pointing at the git directory. We do not need to

[PATCH 7/7] submodule--helper: introduce new update-module-mode helper

2018-08-13 Thread Stefan Beller
This chews off a bit of the shell part of the update command in git-submodule.sh. When writing the C code, keep in mind that the submodule--helper part will go away eventually and we want to have a C function that is able to determine the submodule update strategy, it as a nicety, make

[PATCH 0/7] Resend of sb/submodule-update-in-c

2018-08-13 Thread Stefan Beller
Thanks Brandon for pointing out to use repo_git_path instead of manually constructing the path. That is the only change in this resend. Thanks, Stefan Stefan Beller (7): git-submodule.sh: align error reporting for update mode to use path git-submodule.sh: rename unused variables

[PATCH 1/7] git-submodule.sh: align error reporting for update mode to use path

2018-08-13 Thread Stefan Beller
All other error messages in cmd_update are reporting the submodule based on its path, so let's do that for invalid update modes, too. Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- git-submodule.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git

Re: [PATCH] mingw: enable atomic O_APPEND

2018-08-13 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason writes: > O_APPEND in POSIX is race-free only up to PIPE_MAX bytes written at a > time, which is e.g. 2^12 by default on linux, after that all bets are > off and the kernel is free to interleave different write calls. We are only interested in the use of race-safe append

Re: [PATCH 5/8] diff.c: add set_sign to emit_line_0

2018-08-13 Thread Stefan Beller
On Mon, Aug 13, 2018 at 5:15 AM Johannes Schindelin wrote: > > Hi Stefan, > > On Fri, 10 Aug 2018, Stefan Beller wrote: > > > For now just change the signature, we'll reason about the actual > > change in a follow up patch. > > > > Pass 'set_sign' (which is output before the sign) and 'set' which

Re: [PATCH 2/2] fsck: use oidset for skiplist

2018-08-13 Thread Jeff King
On Mon, Aug 13, 2018 at 09:58:42PM -0400, Jeff King wrote: > > builtin/cat-file.c | 93 +++--- > > 1 file changed, 88 insertions(+), 5 deletions(-) > > By the way, your patch seemed damaged (wouldn't apply, and "am -3" > complained of hand-editing). It

Re: Contributor Summit planning

2018-08-13 Thread Christian Couder
On Mon, Aug 13, 2018 at 7:46 PM, Stefan Beller wrote: > On Mon, Aug 13, 2018 at 9:31 AM Jeff King wrote: >> >> For the past several years, we've held a Git Contributor Summit as part >> of the Git Merge conference. I'd like to get opinions from the community >> to help plan future installments.

Bug? Interactive rebase with reword after conflict

2018-08-13 Thread Dániel Vörös
Hey All, I'm seeing some weird behavior when doing interactive rebase of a single commit with reword if there's a conflict. The rebased commit gets "squashed" into the target commit and is not a child of that. I've asked this question in the git-users group first a few weeks ago, but got no

Re: [PATCH] t5318: use 'test_cmp_bin' to compare commit-graph files

2018-08-13 Thread Derrick Stolee
On 8/12/2018 4:18 PM, SZEDER Gábor wrote: The commit-graph files are binary files, so they should not be compared with 'test_cmp', because that might cause issues on Windows, where 'test_cmp' is a shell function to deal with random LF-CRLF conversions. Use 'test_cmp_bin' instead.

Re: [PATCH] t5318: avoid unnecessary command substitutions

2018-08-13 Thread Derrick Stolee
On 8/12/2018 8:30 PM, SZEDER Gábor wrote: Two tests added in dade47c06c (commit-graph: add repo arg to graph readers, 2018-07-11) prepare the contents of 'expect' files by 'echo'ing the results of command substitutions. That's unncessary, avoid them by directly saving the output of the commands

[PATCH v6 03/21] range-diff: first rudimentary implementation

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin At this stage, `git range-diff` can determine corresponding commits of two related commit ranges. This makes use of the recently introduced implementation of the linear assignment algorithm. The core of this patch is a straight port of the ideas of tbdiff, the

[PATCH v6 10/21] range-diff: do not show "function names" in hunk headers

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin We are comparing complete, formatted commit messages with patches. There are no function names here, so stop looking for them. Signed-off-by: Johannes Schindelin --- range-diff.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/range-diff.c b/range-diff.c

[PATCH v6 06/21] range-diff: right-trim commit messages

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin When comparing commit messages, we need to keep in mind that they are indented by four spaces. That is, empty lines are no longer empty, but have "trailing whitespace". When displaying them in color, that results in those nagging red lines. Let's just right-trim the

[PATCH v6 13/21] color: add the meta color GIT_COLOR_REVERSE

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin This "color" simply reverts background and foreground. It will be used in the upcoming "dual color" mode of `git range-diff`, where we will reverse colors for the -/+ markers and the fragment headers of the "outer" diff. Signed-off-by: Johannes Schindelin --- color.h

[PATCH v6 09/21] range-diff: adjust the output of the commit pairs

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin This not only uses "dashed stand-ins" for "pairs" where one side is missing (i.e. unmatched commits that are present only in one of the two commit ranges), but also adds onelines for the reader's pleasure. This change brings `git range-diff` yet another step closer to

[PATCH v6 08/21] range-diff: suppress the diff headers

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin When showing the diff between corresponding patches of the two branch versions, we have to make up a fake filename to run the diff machinery. That filename does not carry any meaningful information, hence tbdiff suppresses it. So we should, too. Signed-off-by:

[PATCH v6 05/21] range-diff: also show the diff between patches

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin Just like tbdiff, we now show the diff between matching patches. This is a "diff of two diffs", so it can be a bit daunting to read for the beginner. An alternative would be to display an interdiff, i.e. the hypothetical diff which is the result of first reverting the

[PATCH v6 12/21] range-diff: use color for the commit pairs

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin Arguably the most important part of `git range-diff`'s output is the list of commits in the two branches, together with their relationships. For that reason, tbdiff introduced color-coding that is pretty intuitive, especially for unchanged patches (all dim yellow, like

[PATCH v6 11/21] range-diff: add tests

2018-08-13 Thread Thomas Rast via GitGitGadget
From: Thomas Rast These are essentially lifted from https://github.com/trast/tbdiff, with light touch-ups to account for the command now being named `git range-diff`. Apart from renaming `tbdiff` to `range-diff`, only one test case needed to be adjusted: 11 - 'changed message'. The underlying

[PATCH v6 17/21] range-diff: populate the man page

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin The bulk of this patch consists of a heavily butchered version of tbdiff's README written by Thomas Rast and Thomas Gummerer, lifted from https://github.com/trast/tbdiff. Signed-off-by: Johannes Schindelin --- Documentation/git-range-diff.txt | 229

[PATCH v6 20/21] range-diff: make --dual-color the default mode

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin After using this command extensively for the last two months, this developer came to the conclusion that even if the dual color mode still leaves a lot of room for confusion about what was actually changed, the non-dual color mode is substantially worse in that regard.

[PATCH v6 19/21] range-diff: left-pad patch numbers

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin As pointed out by Elijah Newren, tbdiff has this neat little alignment trick where it outputs the commit pairs with patch numbers that are padded to the maximal patch number's width: 1: cafedead = 1: acefade first patch [...] 314: beefeada <

[PATCH v6 14/21] diff: add an internal option to dual-color diffs of diffs

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin When diffing diffs, it can be quite daunting to figure out what the heck is going on, as there are nested +/- signs. Let's make this easier by adding a flag in diff_options that allows color-coding the outer diff sign with inverted colors, so that the preimage and

[PATCH v6 15/21] range-diff: offer to dual-color the diffs

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin When showing what changed between old and new commits, we show a diff of the patches. This diff is a diff between diffs, therefore there are nested +/- signs, and it can be relatively hard to understand what is going on. With the --dual-color option, the preimage and

[PATCH v6 01/21] linear-assignment: a function to solve least-cost assignment problems

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin The problem solved by the code introduced in this commit goes like this: given two sets of items, and a cost matrix which says how much it "costs" to assign any given item of the first set to any given item of the second, assign all items (except when the sets have

[PATCH v6 02/21] Introduce `range-diff` to compare iterations of a topic branch

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin This command does not do a whole lot so far, apart from showing a usage that is oddly similar to that of `git tbdiff`. And for a good reason: the next commits will turn `range-branch` into a full-blown replacement for `tbdiff`. At this point, we ignore tbdiff's color

[PATCH v6 00/21] Add range-diff, a tbdiff lookalike

2018-08-13 Thread Johannes Schindelin via GitGitGadget
The incredibly useful git-tbdiff [https://github.com/trast/tbdiff] tool to compare patch series (say, to see what changed between two iterations sent to the Git mailing list) is slightly less useful for this developer due to the fact that it requires the hungarian and numpy Python packages which

[PATCH v6 04/21] range-diff: improve the order of the shown commits

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin This patch lets `git range-diff` use the same order as tbdiff. The idea is simple: for left-to-right readers, it is natural to assume that the `git range-diff` is performed between an older vs a newer version of the branch. As such, the user is probably more interested

[PATCH v6 07/21] range-diff: indent the diffs just like tbdiff

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin The main information in the `range-diff` view comes from the list of matching and non-matching commits, the diffs are additional information. Indenting them helps with the reading flow. Signed-off-by: Johannes Schindelin --- builtin/range-diff.c | 10 ++ 1

[PATCH v6 18/21] completion: support `git range-diff`

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin Tab completion of `git range-diff` is very convenient, especially given that the revision arguments to specify the commit ranges to compare are typically more complex than, say, what is normally passed to `git log`. Signed-off-by: Johannes Schindelin ---

[PATCH v6 16/21] range-diff --dual-color: skip white-space warnings

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin When displaying a diff of diffs, it is possible that there is an outer `+` before a context line. That happens when the context changed between old and new commit. When that context line starts with a tab (after the space that marks it as context line), our diff

[PATCH v6 21/21] range-diff: use dim/bold cues to improve dual color mode

2018-08-13 Thread Johannes Schindelin via GitGitGadget
From: Johannes Schindelin It *is* a confusing thing to look at a diff of diffs. All too easy is it to mix up whether the -/+ markers refer to the "inner" or the "outer" diff, i.e. whether a `+` indicates that a line was added by either the old or the new diff (or both), or whether the new diff

Re: [PATCH 1/4] diff.c: emit_line_0 to take string instead of first sign

2018-08-13 Thread Johannes Schindelin
Hi Stefan, On Fri, 10 Aug 2018, Stefan Beller wrote: > By providing a string as the first part of the emission we can extend > it later more easily. Thank you for working on this! > While at it, document emit_line_0. > > [...] > > +/* > + * Emits > + * LF > + * if they are present.

Re: [PATCH] t5318: use 'test_cmp_bin' to compare commit-graph files

2018-08-13 Thread SZEDER Gábor
On Mon, Aug 13, 2018 at 1:24 PM Derrick Stolee wrote: > > On 8/12/2018 4:18 PM, SZEDER Gábor wrote: > > The commit-graph files are binary files, so they should not be > > compared with 'test_cmp', because that might cause issues on Windows, > > where 'test_cmp' is a shell function to deal with

Re: [PATCH 2/4] diff.c: add --output-indicator-{new, old, context}

2018-08-13 Thread Johannes Schindelin
Hi Steafn, On Fri, 10 Aug 2018, Stefan Beller wrote: > This will prove useful in range-diff in a later patch as we will be able to > differentiate between adding a new file (that line is starting with +++ > and then the file name) and regular new lines. Very good! > It could also be useful for

Re: [PATCH 3/4] range-diff: make use of different output indicators

2018-08-13 Thread Johannes Schindelin
Hi Stefan, On Fri, 10 Aug 2018, Stefan Beller wrote: > This change itself only changes the internal communication and should > have no visible effect to the user. We instruct the diff code that produces > the inner diffs to use X, Y, Z instead of the usual markers for new, old > and context

Re: [PATCH v6 00/21] Add range-diff, a tbdiff lookalike

2018-08-13 Thread Johannes Schindelin
Hi, On Mon, 13 Aug 2018, Johannes Schindelin via GitGitGadget wrote: > The incredibly useful git-tbdiff [https://github.com/trast/tbdiff] tool to > compare patch series (say, to see what changed between two iterations sent > to the Git mailing list) is slightly less useful for this developer due

Re: [PATCH 4/4] range-diff: indent special lines as context

2018-08-13 Thread Johannes Schindelin
Hi Stefan, On Fri, 10 Aug 2018, Stefan Beller wrote: > The range-diff coloring is a bit fuzzy when it comes to special lines of > a diff, such as indicating new and old files with +++ and ---, as it > would pickup the first character and interpret it for its coloring, which > seems annoying as

[PATCH v2] t5318: use 'test_cmp_bin' to compare commit-graph files

2018-08-13 Thread SZEDER Gábor
The commit-graph files are binary files, so they should not be compared with 'test_cmp', because that might cause issues like crashing[1] or infinite loop[2] on Windows, where 'test_cmp' is a shell function to deal with random LF-CRLF conversions[3]. Use 'test_cmp_bin' instead. 1 - b93e6e3663

Re: [PATCH 1/1] verify-tag/verify-commit should exit unsuccessfully when signature is not trusted

2018-08-13 Thread Vojtech Myslivec
On 9.8.2018 20:40, Junio C Hamano wrote: > Jeff King writes: > >> I guess leaving it serves as a sort of cross-check if gpg would return a >> zero exit code but indicate in the status result that the signature was >> not good. Sort of a belt-and-suspenders, I guess (which might not be >> that

[PATCH v2 5/6] chainlint: recognize multi-line quoted strings more robustly

2018-08-13 Thread Eric Sunshine
chainlint.sed recognizes multi-line quoted strings within subshells: echo "abc def" >out && so it can avoid incorrectly classifying lines internal to the string as breaking the &&-chain. To identify the first line of a multi-line string, it checks if the line contains a single quote.

[PATCH v2 0/6] chainlint: improve robustness against "unusual" shell coding

2018-08-13 Thread Eric Sunshine
This is a re-roll of [1] which improves chainlint's robustness in the face of unusual shell coding such as in contrib/subtree/t7900 which triggered a false-positive[2]. Changes since v1: * recognize lowercase in here-doc tag names (in addition to uppercase) * recognize 'quoted' here-doc tag

[PATCH v2 4/6] chainlint: let here-doc and multi-line string commence on same line

2018-08-13 Thread Eric Sunshine
After swallowing a here-doc, chainlint.sed assumes that no other processing needs to be done on the line aside from checking for &&-chain breakage; likewise, after folding a multi-line quoted string. However, it's conceivable (even if unlikely in practice) that both a here-doc and a multi-line

[PATCH v2 3/6] chainlint: recognize multi-line $(...) when command cuddled with "$("

2018-08-13 Thread Eric Sunshine
For multi-line $(...) expressions nested within subshells, chainlint.sed only recognizes: x=$( echo foo && ... but it is not unlikely that test authors may also cuddle the command with the opening "$(", so support that style, as well: x=$(echo foo && ... The

[PATCH v2 2/6] chainlint: match 'quoted' here-doc tags

2018-08-13 Thread Eric Sunshine
A here-doc tag can be quoted ('EOF') or escaped (\EOF) to suppress interpolation within the body. Although, chainlint recognizes escaped tags, it does not know about quoted tags. For completeness, teach it to recognize quoted tags, as well. Signed-off-by: Eric Sunshine --- t/chainlint.sed

[PATCH v2 6/6] chainlint: add test of pathological case which triggered false positive

2018-08-13 Thread Eric Sunshine
This extract from contrib/subtree/t7900 triggered a false positive due to three chainlint limitations: * recognizing only a "blessed" set of here-doc tag names in a subshell ("EOF", "EOT", "INPUT_END"), of which "TXT" is not a member * inability to recognize multi-line $(...) when the first

[PATCH v2 1/6] chainlint: match arbitrary here-docs tags rather than hard-coded names

2018-08-13 Thread Eric Sunshine
chainlint.sed swallows top-level here-docs to avoid being fooled by content which might look like start-of-subshell. It likewise swallows here-docs in subshells to avoid marking content lines as breaking the &&-chain, and to avoid being fooled by content which might look like end-of-subshell,

Re: [PATCH v5 05/21] range-diff: also show the diff between patches

2018-08-13 Thread Johannes Schindelin
Hi Thomas, On Sun, 12 Aug 2018, Thomas Gummerer wrote: > On 08/10, Johannes Schindelin via GitGitGadget wrote: > > From: Johannes Schindelin > > > > [..] > > > > @@ -13,15 +14,38 @@ NULL > > int cmd_range_diff(int argc, const char **argv, const char *prefix) > > { > > int creation_factor

[RFC] implement branch.sort config option

2018-08-13 Thread Samuel Maftoul
Currently, you can: git tag --sort=$sorting_key You can also do this on branches: git branch --sort=$sorting_key For tags, you can also configure it with a config key: git config tag.sort $sorting_key But there is no corresponding config for sorting branches. Locally, I have a (pretty

Re: [PATCH 2/8] t3206: add color test for range-diff --dual-color

2018-08-13 Thread Johannes Schindelin
Hi Stefan, On Fri, 10 Aug 2018, Stefan Beller wrote: > +test_expect_success 'dual-coloring' ' > + sed -e "s|^:||" >expect <<-\EOF && > + :1: a4b = 1: f686024 s/5/A/ > + :2: f51d370 ! 2: > 4ab067d s/4/A/ > + :@@ -2,6 +2,8 @@ That's a neat trick to have an indented

Re: [PATCH 4/8] diff.c: reorder arguments for emit_line_ws_markup

2018-08-13 Thread Johannes Schindelin
Hi Stefan, On Fri, 10 Aug 2018, Stefan Beller wrote: > The order shall be all colors first, then the content, flags at the end. Okay. > The colors are in order. In order of what? Of the wavelength? (I agree that the order now makes more sense, and that the diff is correct.) Ciao, Dscho >

Re: [PATCH 6/8] diff: use emit_line_0 once per line

2018-08-13 Thread Johannes Schindelin
Hi Stefan, On Fri, 10 Aug 2018, Stefan Beller wrote: > All lines that use emit_line_0 multiple times per line, are combined > into a single call to emit_line_0, making use of the 'set' argument. > > Signed-off-by: Stefan Beller > Signed-off-by: Junio C Hamano > --- > diff.c | 26

Re: [PATCH 8/8] diff.c: rewrite emit_line_0 more understandably

2018-08-13 Thread Johannes Schindelin
Hi Stefan, On Fri, 10 Aug 2018, Stefan Beller wrote: > emit_line_0 has no nested conditions, but puts out all its arguments > (if set) in order. Well, currently `emit_line_0()` *has* nested conditions: `first == '\n'` inside `len == 0`. And these nested conditions make things hard to read, so

Re: [PATCH 3/8] diff.c: simplify caller of emit_line_0

2018-08-13 Thread Johannes Schindelin
Hi Stefan, On Fri, 10 Aug 2018, Stefan Beller wrote: > Due to the previous condition we know "set_sign != NULL" at that point. I trust your judgement on that, also on how likely this previous condition is to keep guaranteeing that assumption. Thank you, Dscho > Signed-off-by: Stefan Beller >

Re: [PATCH 5/8] diff.c: add set_sign to emit_line_0

2018-08-13 Thread Johannes Schindelin
Hi Stefan, On Fri, 10 Aug 2018, Stefan Beller wrote: > For now just change the signature, we'll reason about the actual > change in a follow up patch. > > Pass 'set_sign' (which is output before the sign) and 'set' which > controls the color after the first character. Hence, promote any >

Re: [PATCH v4 1/7] Add delta-islands.{c,h}

2018-08-13 Thread Ramsay Jones
On 13/08/18 04:33, Christian Couder wrote: > On Mon, Aug 13, 2018 at 3:14 AM, Ramsay Jones [snip] >> And neither the sha1 or str hash-maps are destroyed here. >> (That is not necessarily a problem, of course! ;-) ) > > The instances are declared as static: > > static khash_sha1

Re: [PATCH 7/8] diff.c: compute reverse locally in emit_line_0

2018-08-13 Thread Johannes Schindelin
Hi, On Fri, 10 Aug 2018, Stefan Beller wrote: > Signed-off-by: Stefan Beller > Signed-off-by: Junio C Hamano Well, my rationale for having the explicit `reverse` parameter is: this code is complex enough, introducing some magic "this and that implies this" makes it much harder to understand.

Re: [PATCH v4 5/5] unpack-trees: reuse (still valid) cache-tree from src_index

2018-08-13 Thread Elijah Newren
On Sun, Aug 12, 2018 at 1:16 AM Nguyễn Thái Ngọc Duy wrote: > > We do n-way merge by walking the source index and n trees at the same > time and add merge results to a new temporary index called o->result. > The merge result for any given path could be either > > - keep_entry(): same old index

Re: [PATCH v4 5/5] unpack-trees: reuse (still valid) cache-tree from src_index

2018-08-13 Thread Ben Peart
On 8/13/2018 11:48 AM, Elijah Newren wrote: On Sun, Aug 12, 2018 at 1:16 AM Nguyễn Thái Ngọc Duy wrote: We do n-way merge by walking the source index and n trees at the same time and add merge results to a new temporary index called o->result. The merge result for any given path could be

Re: [GSoC][PATCH v6 00/20] rebase -i: rewrite in C

2018-08-13 Thread Duy Nguyen
On Fri, Aug 10, 2018 at 6:54 PM Alban Gruin wrote: > > This patch series rewrite the interactive rebase from shell to C. I was running some tests on 'pu' and ran git-rebase--interactive without arguments because a test failed and I was wondering if it was me. It showed this > ~/w/ztemp $

[PATCH 07/24] ls-files: correct index argument to get_convert_attr_ascii()

2018-08-13 Thread Nguyễn Thái Ngọc Duy
write_eolinfo() does take an istate as function argument and it should be used instead of the_index. Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/ls-files.c | 17 + 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/builtin/ls-files.c b/builtin/ls-files.c index

[PATCH 00/24] Kill the_index part3

2018-08-13 Thread Nguyễn Thái Ngọc Duy
This is the third part of killing the_index (at least outside builtin/). Part 1 [1] is dropped. Part 2 is nd/no-extern on 'pu'. This part is built on top of nd/no-extern. This series would actually break 'pu' because builtin/stash.c uses three functions that are updated here. So we would need

[PATCH 11/24] unpack-trees: convert clear_ce_flags* to avoid the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Prior to fba92be8f7, this code implicitly (and incorrectly) assumes the_index when running the exclude machinery. fba92be8f7 helps show this problem clearer because unpack-trees operation is supposed to work on whatever index the caller specifies... not specifically the_index. Update the code to

[PATCH 09/24] unpack-trees: add a note about path invalidation

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy --- unpack-trees.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/unpack-trees.c b/unpack-trees.c index f9efee0836..c07a6cd646 100644 --- a/unpack-trees.c +++ b/unpack-trees.c @@ -1552,6 +1552,17 @@ static int verify_uptodate_sparse(const

[PATCH] test verify-commit/tag to exit unsuccessfully

2018-08-13 Thread Vojtech Myslivec
Hello There was a discussion in the mailing list with subject 'verify-tag/verify-commit should exit unsuccessfully when signature is not trusted' which leads to handling exit code of untrusted signatures in 4e5dc9ca1. git verify-commit and verify-tag should exit unsuccessfully when processing a

Re: [PATCH v4] clone: report duplicate entries on case-insensitive filesystems

2018-08-13 Thread Jeff Hostetler
On 8/12/2018 5:07 AM, Nguyễn Thái Ngọc Duy wrote: Paths that only differ in case work fine in a case-sensitive filesystems, but if those repos are cloned in a case-insensitive one, you'll get problems. The first thing to notice is "git status" will never be clean with no indication what

Re: [PATCH v4 5/5] unpack-trees: reuse (still valid) cache-tree from src_index

2018-08-13 Thread Duy Nguyen
On Mon, Aug 13, 2018 at 5:48 PM Elijah Newren wrote: > > On Sun, Aug 12, 2018 at 1:16 AM Nguyễn Thái Ngọc Duy > wrote: > > > > We do n-way merge by walking the source index and n trees at the same > > time and add merge results to a new temporary index called o->result. > > The merge result for

[PATCH 16/24] attr: remove index from git_attr_set_direction()

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Since attr checking API now take the index, there's no need to set an index in advance with this call. Most call sites are straightforward because they either pass the_index or NULL (which defaults back to the_index previously). There's only one suspicious call site in unpack-trees.c where it sets

[PATCH 10/24] unpack-trees: don't shadow global var the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
This function mark_new_skip_worktree() has an argument named the_index which is also the name of a global variable. While they have different types (the global the_index is not a pointer) mistakes can easily happen and it's also confusing for readers. Rename the function argument to something

[PATCH 23/24] apply.c: remove implicit dependency on the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Use apply_state->repo->index instead of the_index (in most cases, unless we need to use a temporary index in some functions). Let the callers (am and apply) tell us what to use, instead of always assuming to operate on the_index. Signed-off-by: Nguyễn Thái Ngọc Duy --- apply.c | 46

[PATCH 02/24] cache-tree: wrap the_index based wrappers with #ifdef

2018-08-13 Thread Nguyễn Thái Ngọc Duy
This puts update_main_cache_tree() and write_cache_as_tree() in the same group of "index compat" functions that assume the_index implicitly, which should only be used within builtin/ or t/helper. sequencer.c is also updated to not use these functions. As of now, no files outside builtin/ use

[PATCH 05/24] dir.c: remove an implicit dependency on the_index in pathspec code

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Make the match_patchspec API and friends take an index_state instead of assuming the_index in dir.c. All external call sites are converted blindly to keep the patch simple and retain current behavior. Individual call sites may receive further updates to use the right index instead of the_index.

[PATCH 18/24] archive.c: avoid access to the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy --- archive.c| 45 ++-- archive.h| 16 +++--- builtin/archive.c| 2 +- builtin/upload-archive.c | 3 ++- 4 files changed, 45 insertions(+), 21 deletions(-) diff --git

[PATCH 03/24] attr: remove an implicit dependency on the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Make the attr API take an index_state instead of assuming the_index in attr code. All call sites are converted blindly to keep the patch simple and retain current behavior. Individual call sites may receive further updates to use the right index instead of the_index. There is one ugly temporary

[PATCH 01/24] diff.c: move read_index() code back to the caller

2018-08-13 Thread Nguyễn Thái Ngọc Duy
This code is only needed for diff-tree (since f0c6b2a2fd ([PATCH] Optimize diff-tree -[CM] --stdin - 2005-05-27)). Let the caller do the preparation instead and avoid read_index() in diff.c code. read_index() should be avoided (in addition to the_index) because it uses get_index_file() underneath

[PATCH 06/24] preload-index.c: use the right index instead of the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy --- preload-index.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/preload-index.c b/preload-index.c index d61d7662d5..71cd2437a3 100644 --- a/preload-index.c +++ b/preload-index.c @@ -58,7 +58,7 @@ static void *preload_thread(void *_data)

[PATCH 04/24] convert.c: remove an implicit dependency on the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Make the convert API take an index_state instead of assuming the_index in convert.c. All external call sites are converted blindly to keep the patch simple and retain current behavior. Individual call sites may receive further updates to use the right index instead of the_index. Signed-off-by:

[PATCH 13/24] pathspec.c: use the right index instead of the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy --- pathspec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pathspec.c b/pathspec.c index 897cb9cbbe..6f005996fd 100644 --- a/pathspec.c +++ b/pathspec.c @@ -37,7 +37,7 @@ void add_pathspec_matches_against_index(const struct pathspec

[PATCH 21/24] apply.c: pass struct apply_state to more functions

2018-08-13 Thread Nguyễn Thái Ngọc Duy
we're going to remove the dependency on the_index by moving 'struct index_state *' to somewhere inside struct apply_state. Let's make sure relevant functions have access to this struct now and reduce the diff noise when the actual conversion happens. Signed-off-by: Nguyễn Thái Ngọc Duy ---

[PATCH 12/24] unpack-trees: avoid the_index in verify_absent()

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Both functions that are updated in this commit are called by verify_absent(), which is part of the "unpack-trees" operation that is supposed to work on any index file specified by the caller. Thanks to Brandon [1] [2], an implicit dependency on the_index is exposed. This commit fixes it. In both

[PATCH 19/24] archive-*.c: use the right repository

2018-08-13 Thread Nguyễn Thái Ngọc Duy
With 'struct archive_args' gaining new repository pointer, we don't have to assume the_repository in the archive backends anymore. Signed-off-by: Nguyễn Thái Ngọc Duy --- archive-tar.c | 2 +- archive-zip.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/archive-tar.c

[PATCH 20/24] resolve-undo.c: use the right index instead of the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy --- resolve-undo.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/resolve-undo.c b/resolve-undo.c index d2e2d22b7f..236320f179 100644 --- a/resolve-undo.c +++ b/resolve-undo.c @@ -188,7 +188,7 @@ void unmerge_index(struct index_state

[PATCH 15/24] entry.c: use the right index instead of the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
checkout-index.c needs update because if checkout->istate is NULL, ie_match_stat() will crash. Previously this is ie_match_stat(_index, ..) so it will not crash, but it is not technically correct either. Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/checkout-index.c | 1 + entry.c

[PATCH 17/24] grep: use the right index instead of the_index

2018-08-13 Thread Nguyễn Thái Ngọc Duy
Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/grep.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/grep.c b/builtin/grep.c index 9d7ba87f9b..b7033954ac 100644 --- a/builtin/grep.c +++ b/builtin/grep.c @@ -497,7 +497,7 @@ static int grep_cache(struct grep_opt

[PATCH 22/24] apply.c: make init_apply_state() take a struct repository

2018-08-13 Thread Nguyễn Thái Ngọc Duy
We're moving away from the_index in this code. "struct index_state *" could be added to struct apply_state. But let's aim long term and put struct repository here instead so that we could even avoid more global states in the future. The index will be available via apply_state->repo->index.

<    1   2   3   >