[PATCH v4] doc: do not use `rm .git/index` when normalizing line endings

2017-06-13 Thread Andreas Heiduk
When illustrating how to normalize the line endings, the documentation in gitattributes tells the user to `rm .git/index`. This is incorrect for two reasons: - Users shouldn't be instructed to mess around with the internal implementation of Git using raw file system tools like `rm`. - Withi

Re: [PATCH 1/2] add: warn when adding an embedded repository

2017-06-13 Thread Jeff King
On Tue, Jun 13, 2017 at 10:16:15AM -0700, Junio C Hamano wrote: > Jeff King writes: > > > I also waffled on whether we should ask the submodule code > > whether it knows about a particular path. Technically: > > > > git config submodule.foo.path foo > > git config submodule.foo.url git://...

Re: [PATCH 1/2] add: warn when adding an embedded repository

2017-06-13 Thread Jeff King
On Tue, Jun 13, 2017 at 10:07:43AM -0700, Stefan Beller wrote: > > I also waffled on whether we should ask the submodule code > > whether it knows about a particular path. Technically: > > > > git config submodule.foo.path foo > > git config submodule.foo.url git://... > > git add foo > > >

Re: [PATCH v2 4/6] config: don't implicitly use gitdir

2017-06-13 Thread Jeff King
On Tue, Jun 13, 2017 at 02:38:15PM -0700, Brandon Williams wrote: > > The same comments as before still apply: > > > > - this changes API to make opts->git_dir mandatory, which is error prone > > and easily avoidable, e.g. by making git_dir an argument to > > git_config_with_options > > I st

Re: [PATCH v2 5/6] setup: teach discover_git_directory to respect the commondir

2017-06-13 Thread Jeff King
On Tue, Jun 13, 2017 at 02:03:20PM -0700, Brandon Williams wrote: > Currently 'discover_git_directory' only looks at the gitdir to determine > if a git directory was discovered. This causes a problem in the event > that the gitdir which was discovered was in fact a per-worktree git > directory an

Re: [PATCH v3 6/6] Use the early config machinery to expand aliases

2017-06-13 Thread Jeff King
On Tue, Jun 13, 2017 at 09:21:30AM -0700, Junio C Hamano wrote: > > alias.c | 31 --- > > git.c| 55 > > --- > > t/t7006-pager.sh | 2 +- > > 3 files changed, 29 insertions(+), 59 deletions(-) >

Re: [PATCH v2 7/8] alias_lookup(): optionally return top-level directory

2017-06-13 Thread Jeff King
On Tue, Jun 13, 2017 at 09:40:49AM -0700, Junio C Hamano wrote: > >> It is really tempting to avoid the extra strbuf, but then the error > >> message would change from > >> > >>error: missing value for 'alias.br' > >> > >> to > >> > >>error: missing value for 'br' > >> > >> which is of

Re: [PATCH v3 6/6] Use the early config machinery to expand aliases

2017-06-13 Thread Jeff King
On Tue, Jun 13, 2017 at 11:26:06AM -0700, Brandon Williams wrote: > So because I've been looking at the config machinery lately, I've > noticed a lot of issues with how things are handled with respect to > gitdir vs commondir. Essentially the config resides at commondir/config > always, and only

[PATCH] configure.ac: loosen FREAD_READS_DIRECTORIES test program

2017-06-13 Thread Jeff King
On Wed, Jun 14, 2017 at 01:15:44AM -0400, Jeff King wrote: > > I couldn't reproduce either with my usual build, but I don't usually use > > autoconf. Running: > > > > make configure > > ./configure > > make > > (cd t && ./t1308-*) > > > > does fail for me. The problem is that the generat

Re: t1308-config-set.sh fails on current master

2017-06-13 Thread Jeff King
On Wed, Jun 14, 2017 at 01:02:15AM -0400, Jeff King wrote: > On Wed, Jun 14, 2017 at 04:17:40AM +0200, Øyvind A. Holm wrote: > > > > Interesting. I'm not able to reproduce it, but of course that doesn't > > > mean much. > > > > I'll admit that I have a somewhat special build system, but it's be

Re: t1308-config-set.sh fails on current master

2017-06-13 Thread Jeff King
On Wed, Jun 14, 2017 at 04:17:40AM +0200, Øyvind A. Holm wrote: > > Interesting. I'm not able to reproduce it, but of course that doesn't > > mean much. > > I'll admit that I have a somewhat special build system, but it's been > working great since I created it 7 months ago, and I run the test

Re: [PATCH v2 4/6] config: don't implicitly use gitdir

2017-06-13 Thread Jacob Keller
On Tue, Jun 13, 2017 at 3:05 PM, Jonathan Nieder wrote: > Junio C Hamano wrote: >> On Tue, Jun 13, 2017 at 2:51 PM, Jonathan Nieder wrote: > >>> What is the next step, then? You can find the notion ridiculous but >>> it's how this project has worked in my experience (and how other >>> projects w

Re: [PATCH v3] doc: do not use `rm .git/index` when normalizing line endings

2017-06-13 Thread Torsten Bögershausen
On 14/06/17 00:15, Andreas Heiduk wrote: Looks good to me, one minor typo below When illustrating how to normalize the line endings, the documentation in gitattributes tells the user to `rm .git/index`. This is incorrect for two reasons: - Users shouldn't be instructed to mess around with

Re: [PATCH] send-email: Add tocmd option to suppress-cc

2017-06-13 Thread Viresh Kumar
On 13-06-17, 10:23, Junio C Hamano wrote: > Viresh Kumar writes: > > >> Going back to the core part of your change, i.e. > >> > >> - foreach my $entry (qw (cccmd cc author self sob body bodycc)) { > >> + foreach my $entry (qw (tocmd cccmd cc author self sob body bodycc)) { > >> > >> to think

Re: [PATCHv5 16/17] diff: buffer all output if asked to

2017-06-13 Thread Stefan Beller
On Tue, Jun 13, 2017 at 3:07 PM, Jonathan Tan wrote: >> +struct diff_line { > > Probably should be called diff_emission (or just emission), since these > may not be full lines. I think emitted_string would do as well? > > Also, can this definition be in the .c file? Callers should use the > dif

Re: t1308-config-set.sh fails on current master

2017-06-13 Thread Øyvind A . Holm
Hi, Jonathan, thanks for having a look at this. On 2017-06-13 18:25:35, Jonathan Nieder wrote: > Øyvind A. Holm wrote: > > t1308-config-set.sh fails on current master > > (v2.13.1-449-g02a2850ad58e). The error is introduced by commit > > e2d90fd1c33a ("config.mak.uname: set FREAD_READS_DIRECTORI

Re: t1308-config-set.sh fails on current master

2017-06-13 Thread Jonathan Nieder
Hi Øyvind, Øyvind A. Holm wrote: > t1308-config-set.sh fails on current master (v2.13.1-449-g02a2850ad58e). > The error is introduced by commit e2d90fd1c33a ("config.mak.uname: set > FREAD_READS_DIRECTORIES for Linux and FreeBSD"). Reverting the commit > results in a conflict, but the test wor

Re: proposal for how to share other refs as part of refs/tracking/*

2017-06-13 Thread Jacob Keller
On Tue, Jun 13, 2017 at 8:55 AM, Marc Branchaud wrote: > On 2017-06-13 10:41 AM, Marc Branchaud wrote: >> >> >> So I like your refs/tracking proposal, and hope that it aims for mirroring >> a remote's refs, to eventually replace refs/remotes entirely. > > > To be extra-clear: > > I think a refs/tr

t1308-config-set.sh fails on current master

2017-06-13 Thread Øyvind A . Holm
t1308-config-set.sh fails on current master (v2.13.1-449-g02a2850ad58e). The error is introduced by commit e2d90fd1c33a ("config.mak.uname: set FREAD_READS_DIRECTORIES for Linux and FreeBSD"). Reverting the commit results in a conflict, but the test works on the commit before, 02912f477586. Te

[ANNOUNCE] public-inbox.org/git/ search updates for diffs

2017-06-13 Thread Eric Wong
Hey all, https://public-inbox.org/git/_/text/help has a few new prefixes which might help improve searching: dfn: match filename from diff dfa: match diff removed (-) lines dfb: match diff added (+) lines dfhh:match diff hunk header context (usually

Re: What's cooking in git.git (Jun 2017, #04; Tue, 13)

2017-06-13 Thread Jonathan Nieder
Jun 13, 2017 at 02:40:16PM -0700, Junio C Hamano wrote: > * sb/submodule-blanket-recursive (2017-06-01) 9 commits > (merged to 'next' on 2017-06-04 at 418bb03032) > + builtin/fetch.c: respect 'submodule.recurse' option > + builtin/push.c: respect 'submodule.recurse' option > + builtin/grep.c:

Re: [PATCH] remote: drop free_refspecs() function

2017-06-13 Thread Jonathan Nieder
Jeff King wrote: > Subject: [PATCH] remote: drop free_refspecs() function > > We already have free_refspec(), a public function which does > the same thing as the static free_refspecs(). Let's just > keep one. There are two minor differences between the > functions: > > 1. free_refspecs() is a

Re: [PATCHv4 1/2] clone: respect additional configured fetch refspecs during initial fetch

2017-06-13 Thread Jonathan Nieder
Hi, SZEDER Gábor wrote: > The initial fetch during a clone doesn't transfer refs matching > additional fetch refspecs given on the command line as configuration > variables. This contradicts the documentation stating that > configuration variables specified via 'git clone -c = ...' > "take effec

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Stefan Beller
On Tue, Jun 13, 2017 at 4:42 PM, Jonathan Tan wrote: > On Mon, 12 Jun 2017 19:31:51 -0700 > Stefan Beller wrote: > >> When using git-blame lots of lines contain redundant information, for >> example in hunks that consist of multiple lines, the metadata (commit name, >> author, timezone) are repea

[BUG] b9c8e7f2fb6e breaks git bisect visualize

2017-06-13 Thread Øyvind A . Holm
Commit b9c8e7f2fb6e ("prefix_ref_iterator: don't trim too much") breaks git bisect visualize, this reproduces the bug: $ git bisect start $ git bisect bad $ git bisect good HEAD^^ $ git bisect visualize fatal: BUG: attempt to trim too many characters $ Reverting b9c8e7f2fb6e makes gi

Re: [PATCHv5 04/17] diff: introduce more flexible emit function

2017-06-13 Thread Jonathan Tan
On Tue, 13 Jun 2017 16:41:57 -0700 Stefan Beller wrote: > On Tue, Jun 13, 2017 at 2:54 PM, Jonathan Tan > wrote: > > - could this be called emit() instead? > > Despite having good IDEs available some (including me) > very much like working with raw text, and then having a function > named as a

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Jonathan Tan
On Mon, 12 Jun 2017 19:31:51 -0700 Stefan Beller wrote: > When using git-blame lots of lines contain redundant information, for > example in hunks that consist of multiple lines, the metadata (commit name, > author, timezone) are repeated. A reader may not be interested in those, > so darken them

Re: [PATCHv5 04/17] diff: introduce more flexible emit function

2017-06-13 Thread Stefan Beller
On Tue, Jun 13, 2017 at 2:54 PM, Jonathan Tan wrote: > On Wed, 24 May 2017 14:40:23 -0700 > Stefan Beller wrote: > >> Currently, diff output is written either through the emit_line_0 >> function or through the FILE * in struct diff_options directly. To >> make it easier to teach diff to buffer it

Re: [PATCHv4 2/2] Documentation/clone: document ignored configuration variables

2017-06-13 Thread Jonathan Nieder
Hi, SZEDER Gábor wrote: > Due to limitations/bugs in the current implementation, some > configuration variables specified via 'git clone -c var=val' (or 'git > -c var=val clone') are ignored during the initial fetch and checkout. > > Let the users know which configuration variables are known to b

Re: [PATCH 8/8] t0012: test "-h" with builtins

2017-06-13 Thread Jonathan Nieder
Hi, Jeff King wrote: > --- a/t/t0012-help.sh > +++ b/t/t0012-help.sh > @@ -49,4 +49,16 @@ test_expect_success "--help does not work for guides" " > test_i18ncmp expect actual > " > > +test_expect_success 'generate builtin list' ' > + git --list-builtins >builtins > +' > + > +while rea

Re: [PATCH] diff.c: color moved lines differently

2017-06-13 Thread Jonathan Tan
On Wed, 31 May 2017 17:24:29 -0700 Stefan Beller wrote: > When a patch consists mostly of moving blocks of code around, it can > be quite tedious to ensure that the blocks are moved verbatim, and not > undesirably modified in the move. To that end, color blocks that are > moved within the same pa

Re: [BUG] add_again() off-by-one error in custom format

2017-06-13 Thread SZEDER Gábor
[sorry for double post, forgot the mailing list...] To throw in a fourth option, this one adjusts the expansions' cached offsets when the magic makes it necessary. It's not necessary for '%-', because it only makes a difference when the expansion is empty, and in that case - add_again() doesn'

Re: What's cooking in git.git (Jun 2017, #03; Mon, 5)

2017-06-13 Thread Stefan Beller
On Wed, Jun 7, 2017 at 10:41 PM, Jacob Keller wrote: > On Mon, Jun 5, 2017 at 11:52 PM, Jacob Keller wrote: >> >> I will try to find some time tomorrow to go over it in detail. >> https://public-inbox.org/git/20170613023151.9688-1-sbel...@google.com/ restarted the discussion on this feature, so

[PATCH v3] doc: do not use `rm .git/index` when normalizing line endings

2017-06-13 Thread Andreas Heiduk
When illustrating how to normalize the line endings, the documentation in gitattributes tells the user to `rm .git/index`. This is incorrect for two reasons: - Users shouldn't be instructed to mess around with the internal implementation of Git using raw file system tools like `rm`. - Withi

Re: [PATCHv5 16/17] diff: buffer all output if asked to

2017-06-13 Thread Jonathan Tan
On Wed, 24 May 2017 14:40:35 -0700 Stefan Beller wrote: > diff --git a/diff.h b/diff.h > index 85948ed65a..fad1258556 100644 > --- a/diff.h > +++ b/diff.h > @@ -115,6 +115,42 @@ enum diff_submodule_format { > DIFF_SUBMODULE_INLINE_DIFF > }; > > +/* > + * This struct is used when we need

Re: [PATCH v2 4/6] config: don't implicitly use gitdir

2017-06-13 Thread Jonathan Nieder
Junio C Hamano wrote: > On Tue, Jun 13, 2017 at 2:51 PM, Jonathan Nieder wrote: >> What is the next step, then? You can find the notion ridiculous but >> it's how this project has worked in my experience (and how other >> projects with similar patch-based workflows work). > > Does "patch-based"

Re: [PATCH v2 3/6] config: don't include config.h by default

2017-06-13 Thread Jonathan Nieder
Brandon Williams wrote: > Stop including config.h by default in cache.h. Instead only include > config.h in those files which require use of the config system. > > Signed-off-by: Brandon Williams > --- [...] > 145 files changed, 145 insertions(+), 1 deletion(-) Reviewed-by: Jonathan Nieder Th

Re: [PATCH v2 4/6] config: don't implicitly use gitdir

2017-06-13 Thread Junio C Hamano
On Tue, Jun 13, 2017 at 2:51 PM, Jonathan Nieder wrote: > > What is the next step, then? You can find the notion ridiculous but > it's how this project has worked in my experience (and how other > projects with similar patch-based workflows work). Does "patch-based" have much to do with this? I

Re: [PATCHv5 04/17] diff: introduce more flexible emit function

2017-06-13 Thread Jonathan Tan
On Wed, 24 May 2017 14:40:23 -0700 Stefan Beller wrote: > Currently, diff output is written either through the emit_line_0 > function or through the FILE * in struct diff_options directly. To > make it easier to teach diff to buffer its output (which will be done > in a subsequent commit), introd

Re: [PATCH v2 4/6] config: don't implicitly use gitdir

2017-06-13 Thread Jonathan Nieder
Brandon Williams wrote: > On 06/13, Jonathan Nieder wrote: >> Brandon Williams wrote: >>> Commit 2185fde56 (config: handle conditional include when $GIT_DIR is >>> not set up) added a 'git_dir' field to the config_options struct. Let's >>> use this option field explicitly all the time instead of

What's cooking in git.git (Jun 2017, #04; Tue, 13)

2017-06-13 Thread Junio C Hamano
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. You can find the changes described

Re: [PATCH v2 4/6] config: don't implicitly use gitdir

2017-06-13 Thread Brandon Williams
On 06/13, Jonathan Nieder wrote: > Brandon Williams wrote: > > > Commit 2185fde56 (config: handle conditional include when $GIT_DIR is > > not set up) added a 'git_dir' field to the config_options struct. Let's > > use this option field explicitly all the time instead of occasionally > > falling

Re: [BUG] add_again() off-by-one error in custom format

2017-06-13 Thread Junio C Hamano
René Scharfe writes: > The difference is about the same as the one between: > > $ time git log --format="" >/dev/null > > real0m0.463s > user0m0.448s > sys 0m0.012s > > and: > > $ time git log --format="%h" >/dev/null > > real0m1.062s > us

Re: [PATCH v2 2/6] config: remove git_config_iter

2017-06-13 Thread Jonathan Nieder
Brandon Williams wrote: > Since there is no implementation of the function 'git_config_iter' lets > stop exporting it and remove the prototype from config.h. > > Signed-off-by: Brandon Williams > --- > config.h | 1 - > 1 file changed, 1 deletion(-) Language nit: s/' lets/', let's/ Th

Re: [PATCH v2 1/6] config: create config.h

2017-06-13 Thread Jonathan Nieder
Hi, Brandon Williams wrote: > Move all config related declarations from cache.h to a new config.h > header file. This makes cache.h smaller and allows for the opportunity > in a following patch to only include config.h when needed. > > Signed-off-by: Brandon Williams > --- > cache.h | 190 +--

Re: [PATCH v2 4/6] config: don't implicitly use gitdir

2017-06-13 Thread Jonathan Nieder
Brandon Williams wrote: > Commit 2185fde56 (config: handle conditional include when $GIT_DIR is > not set up) added a 'git_dir' field to the config_options struct. Let's > use this option field explicitly all the time instead of occasionally > falling back to calling 'git_pathdup("config")' to ge

Re: [RFC/PATCH] submodules: overhaul documentation

2017-06-13 Thread Stefan Beller
Adding two native speakers as we start word smithing. On Tue, Jun 13, 2017 at 12:29 PM, Junio C Hamano wrote: >> + >> +A submodule is another Git repository tracked in a subdirectory of your >> +repository. The tracked repository has its own history, which does not >> +interfere with the history

[PATCH v2 4/4] sha1_file, fsck: add missing blob support

2017-06-13 Thread Jonathan Tan
Currently, Git does not support repos with very large numbers of blobs or repos that wish to minimize manipulation of certain blobs (for example, because they are very large) very well, even if the user operates mostly on part of the repo, because Git is designed on the assumption that every blob r

[PATCH v2 0/4] Improvements to sha1_file

2017-06-13 Thread Jonathan Tan
Peff suggested putting in a new field in struct object_info for the object contents; I tried it and it seems to work out quite well. Patch 1 is unmodified from the previous version. Patches 2-3 have been rewritten, and patch 4 is similar except that the missing-lookup change is made to sha1_object

[PATCH v2 3/4] sha1_file: consolidate storage-agnostic object fns

2017-06-13 Thread Jonathan Tan
In sha1_file.c, there are a few functions that provide information on an object regardless of its storage (cached, loose, or packed). Looking through all non-static functions in sha1_file.c that take in an unsigned char * pointer, the relevant ones are: - sha1_object_info_extended - sha1_object_i

[PATCH v2 1/4] sha1_file: teach packed_object_info about typename

2017-06-13 Thread Jonathan Tan
In commit 46f0344 ("sha1_file: support reading from a loose object of unknown type", 2015-05-06), "struct object_info" gained a "typename" field that could represent a type name from a loose object file, whether valid or invalid, as opposed to the existing "typep" which could only represent valid t

[PATCH v2 2/4] sha1_file: move delta base cache code up

2017-06-13 Thread Jonathan Tan
In a subsequent patch, packed_object_info() will be modified to use the delta base cache, so move the relevant code to before packed_object_info(). Signed-off-by: Jonathan Tan --- sha1_file.c | 226 +++- 1 file changed, 116 insertions(+), 1

[PATCH v2 6/6] config: respect commondir

2017-06-13 Thread Brandon Williams
Worktrees present an interesting problem when it comes to the config. Historically we could assume that the per-repository config lives at 'gitdir/config', but since worktrees were introduced this isn't the case anymore. There is currently no way to specify per-worktree configuration, and as such

[PATCH v2 3/6] config: don't include config.h by default

2017-06-13 Thread Brandon Williams
Stop including config.h by default in cache.h. Instead only include config.h in those files which require use of the config system. Signed-off-by: Brandon Williams --- advice.c | 1 + alias.c | 1 + apply.c | 1 + archive

[PATCH v2 4/6] config: don't implicitly use gitdir

2017-06-13 Thread Brandon Williams
Commit 2185fde56 (config: handle conditional include when $GIT_DIR is not set up) added a 'git_dir' field to the config_options struct. Let's use this option field explicitly all the time instead of occasionally falling back to calling 'git_pathdup("config")' to get the path to the local repositor

[PATCH v2 2/6] config: remove git_config_iter

2017-06-13 Thread Brandon Williams
Since there is no implementation of the function 'git_config_iter' lets stop exporting it and remove the prototype from config.h. Signed-off-by: Brandon Williams --- config.h | 1 - 1 file changed, 1 deletion(-) diff --git a/config.h b/config.h index f7f8b66c5..c70599bd5 100644 --- a/config.h +

[PATCH v2 5/6] setup: teach discover_git_directory to respect the commondir

2017-06-13 Thread Brandon Williams
Currently 'discover_git_directory' only looks at the gitdir to determine if a git directory was discovered. This causes a problem in the event that the gitdir which was discovered was in fact a per-worktree git directory and not the common git directory. This is because the repository config, whi

[PATCH v2 0/6] config.h

2017-06-13 Thread Brandon Williams
Changes in v2: * Fix a small nit in builtin/config.c that Jonathan pointed out. * Added two patches which ensure that the repository wide config is properly read by providing 'commondir' as a field in the 'config_options' struct. Brandon Williams (6): config: create config.h config: remov

[PATCH v2 1/6] config: create config.h

2017-06-13 Thread Brandon Williams
Move all config related declarations from cache.h to a new config.h header file. This makes cache.h smaller and allows for the opportunity in a following patch to only include config.h when needed. Signed-off-by: Brandon Williams --- cache.h | 190 +-

Re: [PATCH v4 3/5] stash: add test for stashing in a detached state

2017-06-13 Thread Junio C Hamano
Joel Teichroeb writes: >>> +test_expect_success 'create in a detached state' ' >>> + test_when_finished "git checkout master" && >>> + git checkout HEAD~1 && >>> + >foo && >>> + git add foo && >>> + STASH_ID=$(git stash create) && >>> + HEAD_ID=$(git rev-parse --short HEAD

Re: [BUG] add_again() off-by-one error in custom format

2017-06-13 Thread René Scharfe
Am 13.06.2017 um 20:29 schrieb Junio C Hamano: René Scharfe writes: Indeed, a very nice bug report! I think the call to format_commit_one() needs to be taught to pass 'magic' through, and then add_again() mechanism needs to be told not to cache when magic is in effect, or something like that

Re: [PATCH v4 2/5] stash: Add a test for when apply fails during stash branch

2017-06-13 Thread Joel Teichroeb
On Tue, Jun 13, 2017 at 12:40 PM, Junio C Hamano wrote: > Joel Teichroeb writes: > >> If the return value of merge recurisve is not checked, the stash could end >> up being dropped even though it was not applied properly > > s/recurisve/recursive/ > >> Signed-off-by: Joel Teichroeb >> --- >> t/

Re: [PATCH v4 3/5] stash: add test for stashing in a detached state

2017-06-13 Thread Joel Teichroeb
On Tue, Jun 13, 2017 at 12:45 PM, Junio C Hamano wrote: > Joel Teichroeb writes: > >> Signed-off-by: Joel Teichroeb >> --- >> t/t3903-stash.sh | 12 >> 1 file changed, 12 insertions(+) >> >> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh >> index 5399fb05ca..ce4c8fe3d6 100755 >>

Re: [PATCH v4 4/5] merge: close the index lock when not writing the new index

2017-06-13 Thread Junio C Hamano
Joel Teichroeb writes: > If the merge does not have anything to do, it does not unlock the index, > causing any further index operations to fail. Thus, always unlock the index > regardless of outcome. > > Signed-off-by: Joel Teichroeb > --- This one makes sense. So far, nobody who calls this

Re: [PATCH v4 3/5] stash: add test for stashing in a detached state

2017-06-13 Thread Junio C Hamano
Joel Teichroeb writes: > Signed-off-by: Joel Teichroeb > --- > t/t3903-stash.sh | 12 > 1 file changed, 12 insertions(+) > > diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh > index 5399fb05ca..ce4c8fe3d6 100755 > --- a/t/t3903-stash.sh > +++ b/t/t3903-stash.sh > @@ -822,6 +822,18

Re: [PATCH v4 2/5] stash: Add a test for when apply fails during stash branch

2017-06-13 Thread Junio C Hamano
Joel Teichroeb writes: > If the return value of merge recurisve is not checked, the stash could end > up being dropped even though it was not applied properly s/recurisve/recursive/ > Signed-off-by: Joel Teichroeb > --- > t/t3903-stash.sh | 14 ++ > 1 file changed, 14 insertions(+

Re: [PATCH v4 1/5] stash: add test for stash create with no files

2017-06-13 Thread Junio C Hamano
Joel Teichroeb writes: > Ensure the command gives the correct return code OK. When you know what the correct return code is, we'd prefer to see it spelled out, i.e. Ensure that the command succeeds. Or did you mean that the command outputs nothing? The test itself looks obviously correct

Re: [RFC/PATCH] submodules: overhaul documentation

2017-06-13 Thread Junio C Hamano
Stefan Beller writes: > @@ -149,15 +119,17 @@ deinit [-f|--force] (--all|[--] ...):: > tree. Further calls to `git submodule update`, `git submodule foreach` > and `git submodule sync` will skip any unregistered submodules until > they are initialized again, so use this command

Re: [BUG] add_again() off-by-one error in custom format

2017-06-13 Thread Junio C Hamano
René Scharfe writes: > Indeed, a very nice bug report! > >> I think the call to format_commit_one() needs to be taught to pass >> 'magic' through, and then add_again() mechanism needs to be told not >> to cache when magic is in effect, or something like that. >> >> Perhaps something along this li

Re: [PATCH v3 6/6] Use the early config machinery to expand aliases

2017-06-13 Thread Brandon Williams
On 06/13, Johannes Schindelin wrote: > Instead of discovering the .git/ directory, read the config and then > trying to painstakingly reset all the global state if we did not find a > matching alias, let's use the early config machinery instead. > > It may look like unnecessary work to discover th

Re: [BUG] add_again() off-by-one error in custom format

2017-06-13 Thread René Scharfe
Am 13.06.2017 um 00:49 schrieb Junio C Hamano: Michael Giuffrida writes: For the abbreviated commit hash placeholder ('h'), pretty.c uses add_again() to cache the result of the expansion, and then uses that result in future expansions. This causes problems when the expansion includes whitespac

Re: [PATCH/RFC] branch: add tests for new copy branch feature

2017-06-13 Thread Jonathan Nieder
Ævar Arnfjörð Bjarmason wrote: > On Tue, Jun 13 2017, Jonathan Nieder jotted: > > Ævar Arnfjörð Bjarmason wrote: >>> My understanding of that last part is that Jonathan/someone (see >>> reported-by in that patch) had some script which was renaming >>> branches, and it was easier for whatever reaso

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Junio C Hamano
Stefan Beller writes: > On Tue, Jun 13, 2017 at 10:33 AM, Junio C Hamano wrote: >> Stefan Beller writes: >> >>> But you do not want to (yet)? The goal is not to tell you where the bounds >>> are, but the goal is to point out that extra care is required for review of >>> these particular 3 lines

Re: [PATCH/RFC] branch: add tests for new copy branch feature

2017-06-13 Thread Ævar Arnfjörð Bjarmason
On Tue, Jun 13 2017, Jonathan Nieder jotted: > Hi, > > Ævar Arnfjörð Bjarmason wrote: > >> So the reason we have this for -m is: >> >> commit 3f59481e33 >> Author: Jonathan Nieder >> Date: Fri Nov 25 20:30:02 2011 -0600 >> >> branch: allow a no-op "branch -M HEAD" >> >> Ov

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Stefan Beller
On Tue, Jun 13, 2017 at 10:48 AM, Junio C Hamano wrote: > > I never said "start and end" (you did). I just wanted the boundary > of A and B and C clear, so I'd be perfectly happy with: > > context > +A dim > +A dim > +A highlight #1 > +C

Re: [PATCH/RFC] branch: add tests for new copy branch feature

2017-06-13 Thread Jonathan Nieder
Hi, Ævar Arnfjörð Bjarmason wrote: > So the reason we have this for -m is: > > commit 3f59481e33 > Author: Jonathan Nieder > Date: Fri Nov 25 20:30:02 2011 -0600 > > branch: allow a no-op "branch -M HEAD" > > Overwriting the current branch with a different commit is forbid

Re: [PATCH 2/3] branch: add test for -m renaming multiple config sections

2017-06-13 Thread Ævar Arnfjörð Bjarmason
On Tue, Jun 13 2017, Junio C. Hamano jotted: > Ævar Arnfjörð Bjarmason writes: > >> On Tue, Jun 13, 2017 at 7:10 PM, Junio C Hamano wrote: >>> Indenting using <<- would make it easier to read. I.e. >>> >>> cat >config.branch <<-\EOF && >>> ;; Comment for ... >>> [branch

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Junio C Hamano
Stefan Beller writes: > On Tue, Jun 13, 2017 at 10:33 AM, Junio C Hamano wrote: >> Stefan Beller writes: >> >>> But you do not want to (yet)? The goal is not to tell you where the bounds >>> are, but the goal is to point out that extra care is required for review of >>> these particular 3 lines

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Stefan Beller
On Tue, Jun 13, 2017 at 10:33 AM, Junio C Hamano wrote: > Stefan Beller writes: > >> But you do not want to (yet)? The goal is not to tell you where the bounds >> are, but the goal is to point out that extra care is required for review of >> these particular 3 lines. > > And when you _can_ help u

Re: [PATCH 2/3] branch: add test for -m renaming multiple config sections

2017-06-13 Thread Junio C Hamano
Ævar Arnfjörð Bjarmason writes: > On Tue, Jun 13, 2017 at 7:10 PM, Junio C Hamano wrote: >> Indenting using <<- would make it easier to read. I.e. >> >> cat >config.branch <<-\EOF && >> ;; Comment for ... >> [branch "source"] >> ;; Comment for ... >>

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Junio C Hamano
Stefan Beller writes: > But you do not want to (yet)? The goal is not to tell you where the bounds > are, but the goal is to point out that extra care is required for review of > these particular 3 lines. And when you _can_ help users in that "extra care" by pointing out where the boundary is, w

Re: [PATCH 2/3] branch: add test for -m renaming multiple config sections

2017-06-13 Thread Ævar Arnfjörð Bjarmason
On Tue, Jun 13, 2017 at 7:10 PM, Junio C Hamano wrote: > Sahil Dua writes: > >> + cat >expect <<-\EOF && >> + branch.dest.key1=value1 >> + some.gar.b=age >> + branch.dest.key2=value2 >> + EOF >> + cat >config.branch <<\EOF && >> +;; Comment for source >> +[branch "source"]

Re: [PATCH 3/3] branch: add a --copy (-c) option to go with --move (-m)

2017-06-13 Thread Junio C Hamano
Junio C Hamano writes: > Sahil Dua writes: > >> Add the ability to --copy a branch and its reflog and configuration, >> this uses the same underlying machinery as the --move (-m) option >> except the reflog and configuration is copied instead of being moved. >> >> This is useful for e.g. copying

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Stefan Beller
On Tue, Jun 13, 2017 at 10:19 AM, Junio C Hamano wrote: > Stefan Beller writes: > >> Here is what currently happens: >> >>> >>> context >>> -B dim oldMoved >>> -B dim oldMoved >>> -B highlight oldMovedAlternative >>>

Re: [PATCH] send-email: Add tocmd option to suppress-cc

2017-06-13 Thread Junio C Hamano
Viresh Kumar writes: >> Going back to the core part of your change, i.e. >> >> -foreach my $entry (qw (cccmd cc author self sob body bodycc)) { >> +foreach my $entry (qw (tocmd cccmd cc author self sob body bodycc)) { >> >> to think about it a bit more, I notice that all the existing on

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Junio C Hamano
Stefan Beller writes: > Here is what currently happens: > >> >> context >> -B dim oldMoved >> -B dim oldMoved >> -B highlight oldMovedAlternative >> -A highlight oldMovedAlternative >> -A

Re: [PATCH 1/2] add: warn when adding an embedded repository

2017-06-13 Thread Junio C Hamano
Jeff King writes: > I also waffled on whether we should ask the submodule code > whether it knows about a particular path. Technically: > > git config submodule.foo.path foo > git config submodule.foo.url git://... > git add foo > > is legal, but would still warn with this patch. Did you

Re: [PATCH 1/2] add: warn when adding an embedded repository

2017-06-13 Thread Brandon Williams
On 06/13, Stefan Beller wrote: > On Tue, Jun 13, 2017 at 2:24 AM, Jeff King wrote: > > > There is a config knob that can disable the (long) hint. But > > I intentionally omitted a config knob to disable the warning > > entirely. Whether the warning is sensible or not is > > generally about contex

Re: [PATCH 2/2] t: move "git add submodule" into test blocks

2017-06-13 Thread Stefan Beller
On Tue, Jun 13, 2017 at 2:24 AM, Jeff King wrote: > Some submodule tests do some setup outside of a test_expect > block. This is bad because we won't actually check the > outcome of those commands. But it's doubly so because "git > add submodule" now produces a warning to stderr, which is > not su

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Stefan Beller
On Tue, Jun 13, 2017 at 10:00 AM, Junio C Hamano wrote: > Stefan Beller writes: > >> * As you have an individual color setup, maybe you can fix this >> for you by setting the appropriate slots to your perception of >> dimmed? > > I do not think it is possible with only {new,old}{,alternative}

Re: [PATCH 2/3] branch: add test for -m renaming multiple config sections

2017-06-13 Thread Junio C Hamano
Sahil Dua writes: > + cat >expect <<-\EOF && > + branch.dest.key1=value1 > + some.gar.b=age > + branch.dest.key2=value2 > + EOF > + cat >config.branch <<\EOF && > +;; Comment for source > +[branch "source"] > + ;; Comment for the source value > + key1 = value1 > +

Re: [PATCH 1/3] config: create a function to format section headers

2017-06-13 Thread Ævar Arnfjörð Bjarmason
On Tue, Jun 13, 2017 at 6:17 PM, Sahil Dua wrote: > Factor out the logic which creates section headers in the config file, > e.g. the 'branch.foo' key will be turned into '[branch "foo"]'. +CC Junio: This is to be applied, Sahil is using submitgit which apparently doesn't CC you by default (I don

Re: [PATCH 1/2] add: warn when adding an embedded repository

2017-06-13 Thread Stefan Beller
On Tue, Jun 13, 2017 at 2:24 AM, Jeff King wrote: > It's an easy mistake to add a repository inside another > repository, like: > > git clone $url > git add . > > The resulting entry is a gitlink, but there's no matching > .gitmodules entry. Trying to use "submodule init" (or clone > with --re

Re: [PATCH 1/3] config: create a function to format section headers

2017-06-13 Thread Junio C Hamano
Sahil Dua writes: > Factor out the logic which creates section headers in the config file, > e.g. the 'branch.foo' key will be turned into '[branch "foo"]'. > > This introduces no function changes, but is needed for a later change > which adds support for copying branch sections in the config fil

Re: [PATCH 4/4] config: don't implicitly use gitdir

2017-06-13 Thread Jonathan Nieder
Jeff King wrote: > On Mon, Jun 12, 2017 at 06:38:17PM -0700, Jonathan Nieder wrote: >> Brandon Williams wrote: >>> On 06/12, Jonathan Nieder wrote: Alternatively, could this patch rename git_config_with_options? That way any other patch in flight that calls git_config_with_options would

Re: [PATCH 3/3] branch: add a --copy (-c) option to go with --move (-m)

2017-06-13 Thread Junio C Hamano
Sahil Dua writes: > Add the ability to --copy a branch and its reflog and configuration, > this uses the same underlying machinery as the --move (-m) option > except the reflog and configuration is copied instead of being moved. > > This is useful for e.g. copying a topic branch to a new version,

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Junio C Hamano
Stefan Beller writes: > * As you have an individual color setup, maybe you can fix this > for you by setting the appropriate slots to your perception of > dimmed? I do not think it is possible with only {new,old}{,alternative} 4 colors. Consider this diff: context -B

Re: [PATCH v2 7/8] alias_lookup(): optionally return top-level directory

2017-06-13 Thread Junio C Hamano
Jeff King writes: > On Tue, Jun 13, 2017 at 01:42:02PM +0200, Johannes Schindelin wrote: > >> > As you probably guessed, I had tried that first and then figured that if >> > I needed to keep the config_key_is_valid() test anyway, I could just as >> > well keep the strbuf around for later use. >>

Re: [PATCH v3 6/6] Use the early config machinery to expand aliases

2017-06-13 Thread Junio C Hamano
Johannes Schindelin writes: > Instead of discovering the .git/ directory, read the config and then > trying to painstakingly reset all the global state if we did not find a > matching alias, let's use the early config machinery instead. s/read/&ing/, I think. My reading hiccupped while trying t

Re: [RFC/PATCH] builtin/blame: darken redundant line information

2017-06-13 Thread Stefan Beller
On Tue, Jun 13, 2017 at 8:25 AM, Junio C Hamano wrote: > Stefan Beller writes: > >> When using git-blame lots of lines contain redundant information, for >> example in hunks that consist of multiple lines, the metadata (commit name, >> author, timezone) are repeated. A reader may not be intereste

  1   2   >