Re: `git status` output is very misleading after a merge on a "detached HEAD"

2017-04-12 Thread Michael J Gruber
Ævar Arnfjörð Bjarmason venit, vidit, dixit 12.04.2017 14:18: > On Wed, Apr 12, 2017 at 7:43 AM, Michael J Gruber <g...@grubix.eu> wrote: >> Am 11. April 2017 22:40:14 MESZ schrieb "Ævar Arnfjörð Bjarmason" >> <ava...@gmail.com>: >>> On Tue, Apr

Re: `git status` output is very misleading after a merge on a "detached HEAD"

2017-04-12 Thread Michael J Gruber
Enis Bayramoğlu venit, vidit, dixit 12.04.2017 08:15: > On Wed, Apr 12, 2017 at 8:43 AM, Michael J Gruber <g...@grubix.eu> wrote: >> Am 11. April 2017 22:40:14 MESZ schrieb "Ævar Arnfjörð Bjarmason" >> <ava...@gmail.com>: >>> On Tue, A

Re: `git status` output is very misleading after a merge on a "detached HEAD"

2017-04-11 Thread Michael J Gruber
} else { >branch_name = ""; > on_what = _("Not currently on any branch."); > > > No way. That would reduce the information that we give. Note that the difference between from and at is also: are th

Re: `git status` output is very misleading after a merge on a "detached HEAD"

2017-04-11 Thread Michael J Gruber
tus` simply reports > "HEAD detached from origin/master" while this simply happens to be a > mildly relevant fact about some past state. > > Thanks and regards > Well, what do you suggest as an alternative? Git tells you that you are in detached state and where you came from (detached from). Michael

Re: [PATCH] status: show in-progress info for short status

2017-04-07 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 31.03.2017 20:14: > Michael J Gruber <g...@grubix.eu> writes: > >> Ordinary (long) status shows information about bisect, revert, am, >> rebase, cherry-pick in progress, and so does git-prompt.sh. status >> --short currently

Re: [PATCH] status: show in-progress info for short status

2017-04-07 Thread Michael J Gruber
for almost two > years now... > > git-prompt.sh's similar long conditional chain to show the ongoing > operation has an else-branch at the end showing "AM/REBASE". Your > patch doesn't add an equivalent branch. Is this intentional or an > oversight? I go over all states that wt_status_get_state can give. > I suppose it's intentional, because that "AM/REBASE" branch in the > prompt seems to be unreachable (see below), but I never took the > effort to actually check that (hence the "seems" and that's why I > never submitted it). Note that wt_status_get_state and the prompt script do things quite differently. I suppose that the prompt could make use of the in-progress info being exposed by "git status" rather doing its on thing. Michael

Re: [PATCH] Documentation/git-worktree: use working tree for trees on the file system

2017-04-07 Thread Michael J Gruber
Duy Nguyen venit, vidit, dixit 25.03.2017 13:07: > On Fri, Mar 24, 2017 at 12:55 AM, Junio C Hamano <gits...@pobox.com> wrote: >> Michael J Gruber <g...@drmicha.warpmail.net> writes: >> >>> Are we at a point where we can still rename the new feature at least

Re: [PATCH v7 4/5] dir_iterator: refactor state machine model

2017-04-04 Thread Michael Haggerty
Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- Daniel, > Although it does make tests harder to understand, if we were to > specify how to iterate with human-readable flags we'd add the getopt() > + flag configuration overhead to this helper program to be able to >

Re: [PATCH v3 3/4] name-rev: provide debug output

2017-04-03 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 31.03.2017 20:33: > Junio C Hamano <gits...@pobox.com> writes: > >> Michael J Gruber <g...@grubix.eu> writes: >> >>>> The only case that this change may make a difference I can think of >>>> is when y

Re: [PATCH v7 5/5] remove_subtree(): reimplement using iterators

2017-04-02 Thread Michael Haggerty
nt create_file(const char *path, unsigned int mode) > @@ -312,7 +298,7 @@ int checkout_entry(struct cache_entry *ce, > return 0; > if (!state->force) > return error("%s is a directory", path.buf); > - remove_subtree(); > + remove_subtree(path.buf); > } else if (unlink(path.buf)) > return error_errno("unable to unlink old '%s'", > path.buf); > } else if (state->not_new) > That's a gratifying decrease in code size :-) Michael

Re: [PATCH v7 4/5] dir_iterator: refactor state machine model

2017-04-02 Thread Michael Haggerty
>dir1/a/e && >dir1/d/e/d/a && mkdir -p dir2/a/b/c/ && >dir2/a/b/c/d ' Note that at least one of the test directories has to be renamed so that they don't conflict with each other. There is no need for the individual tests to delete their test directories; the test harness will take care of that. But if you *did* need to clean up after a test, you should do it like this: mkdir foo && test_when_finished "rm -rf foo" && ...tests involving foo... The advantage of `test_when_finished` is that it ensures that the cleanup code is run even if some part of the test fails. > cat >expect-pre-order-output <<-\EOF && > [d] (a) ./dir/a > [d] (a/b) ./dir/a/b > @@ -41,14 +57,53 @@ cat >expect-pre-order-output <<-\EOF && > [f] (a/b/c/d) ./dir/a/b/c/d > EOF > > -test_expect_success 'dir-iterator should list files in the correct order' ' > +test_expect_success 'dir-iterator should list files properly on pre-order > mode' ' > mkdir -p dir/a/b/c/ && > >dir/a/b/c/d && > > - test-dir-iterator ./dir >actual-pre-order-output && > + test-dir-iterator ./dir 1 >actual-pre-order-output && > rm -rf dir && > > test_cmp expect-pre-order-output actual-pre-order-output > ' > > +cat >expect-post-order-output <<-\EOF && > +[f] (a/b/c/d) ./dir/a/b/c/d > +[d] (a/b/c) ./dir/a/b/c > +[d] (a/b) ./dir/a/b > +[d] (a) ./dir/a > +EOF > + > +test_expect_success 'dir-iterator should list files properly on post-order > mode' ' > + mkdir -p dir/a/b/c/ && > + >dir/a/b/c/d && > + > + test-dir-iterator ./dir 2 >actual-post-order-output && > + rm -rf dir && > + > + test_cmp expect-post-order-output actual-post-order-output > +' > + > +cat >expect-pre-order-post-order-root-dir-output <<-\EOF && > +[d] (.) ./dir > +[d] (a) ./dir/a > +[d] (a/b) ./dir/a/b > +[d] (a/b/c) ./dir/a/b/c > +[f] (a/b/c/d) ./dir/a/b/c/d > +[d] (a/b/c) ./dir/a/b/c > +[d] (a/b) ./dir/a/b > +[d] (a) ./dir/a > +[d] (.) ./dir > +EOF > + > +test_expect_success 'dir-iterator should list files properly on pre-order + > post-order + root-dir mode' ' > + mkdir -p dir/a/b/c/ && > + >dir/a/b/c/d && > + > + test-dir-iterator ./dir 7 >actual-pre-order-post-order-root-dir-output > && > + rm -rf dir && > + > + test_cmp expect-pre-order-post-order-root-dir-output > actual-pre-order-post-order-root-dir-output > +' > + > test_done Michael

Re: [PATCH v6 4/5] dir_iterator: refactor state machine model

2017-04-01 Thread Michael Haggerty
es it easier for casual readers to fetch the code and/or look at it online, and also makes it 100% clear what branch point I've chosen. This is by no means obligatory but I find it helpful. There doesn't seem to be much point reviewing broken code, so I'll wait for your feedback and/or fix. Michael [1] https://github.com/mhagger/git-test

Re: [PATCH v2 00/20] Separate `ref_cache` into a separate module

2017-03-31 Thread Michael Haggerty
On 03/31/2017 06:01 PM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> This version literally only contains a few commit message changes and >> one minor comment changes relative to v1. The code is identical. I >> wasn't sure wh

Re: [PATCH v7 00/28] Remove submodule from files-backend.c

2017-03-31 Thread Michael Haggerty
lly, and send a couple of minor comments. But with or without changes, it looks good to me and the whole series is Reviewed-by: Michael Haggerty <mhag...@alum.mit.edu> Thanks! Michael

Re: [PATCH v7 23/28] files-backend: avoid ref api targetting main ref store

2017-03-31 Thread Michael Haggerty
sting is required before that can > happen. Well, except peel_ref(). I'm pretty sure that function is safe. > [...] Michael

Re: [PATCH v7 22/28] refs: new transaction related ref-store api

2017-03-31 Thread Michael Haggerty
differ) rather than a concrete problem. I haven't yet had any difficulties working with your interface. But I wanted to mention my observation anyway. As far as I'm concerned, you don't need to change it. > [...] Michael [*] The name could obviously be improved, but you get the idea.

Re: [PATCH v7 19/28] files-backend: replace submodule_allowed check in files_downcast()

2017-03-31 Thread Michael Haggerty
t; refs = (struct files_ref_store *)ref_store; > > - if (!submodule_allowed) > - files_assert_main_repository(refs, caller); > + if ((refs->store_flags & required_flags) != required_flags) > + die("BUG: unallowed operation (%s), requires %x, has %x\n", > + caller, required_flags, refs->store_flags); Same comment about "unallowed". Maybe BUG: operation %s requires abilities 0x%x but only have 0x%x > > return refs; > } > [...] Michael

Re: [PATCH v3 3/4] name-rev: provide debug output

2017-03-31 Thread Michael J Gruber
Am 31. März 2017 18:52:16 MESZ schrieb Junio C Hamano <gits...@pobox.com>: >Michael J Gruber <g...@grubix.eu> writes: > >> Currently, `git describe --contains --debug` does not create any >debug >> output because it does not pass the flag down to `git name-rev`,

[PATCH v2 13/20] refs: handle "refs/bisect/" in `loose_fill_ref_dir()`

2017-03-31 Thread Michael Haggerty
That "refs/bisect/" has to be handled specially when filling the ref_cache for loose references is a peculiarity of the files backend, and the ref-cache code shouldn't need to know about it. So move this code to the callback function, `loose_fill_ref_dir()`. Signed-off-by: Michael Hagg

[PATCH v2 18/20] commit_packed_refs(): use reference iteration

2017-03-31 Thread Michael Haggerty
-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 38 +- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 736a6c9ff7..0ea42826c8 100644 --- a/refs/files-backend.c +++ b/refs

[PATCH v2 17/20] cache_ref_iterator_begin(): make function smarter

2017-03-31 Thread Michael Haggerty
ator_begin()` to be made more ignorant of the internals of `ref_cache`, and `find_containing_dir()` and `prime_ref_dir()` to be made private. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 44 +--- refs/ref-

[PATCH v2 14/20] do_for_each_entry_in_dir(): eliminate `offset` argument

2017-03-31 Thread Michael Haggerty
It was never used. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 4 ++-- refs/ref-cache.c | 6 +++--- refs/ref-cache.h | 11 +-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/refs/files-backend.c b/refs/files-bac

[PATCH v2 20/20] do_for_each_entry_in_dir(): delete function

2017-03-31 Thread Michael Haggerty
Its only remaining caller was itself. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/ref-cache.c | 21 - refs/ref-cache.h | 11 --- 2 files changed, 32 deletions(-) diff --git a/refs/ref-cache.c b/refs/ref-cache.c index b3a30350d7..6059362f1d

[PATCH v2 10/20] ref-cache: introduce a new type, ref_cache

2017-03-31 Thread Michael Haggerty
For now, it just wraps a `ref_entry *` that points at the root of the tree. Soon it will hold more information. Add two new functions, `create_ref_cache()` and `free_ref_cache()`. Make `free_ref_entry()` private. Change files-backend to use this type to hold its caches. Signed-off-by: Michael

[PATCH v2 11/20] refs: record the ref_store in ref_cache, not ref_dir

2017-03-31 Thread Michael Haggerty
the size of every `ref_dir` instance. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- refs/ref-cache.c | 12 +++- refs/ref-cache.h | 9 ++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/refs/files-backe

[PATCH v2 19/20] files_pack_refs(): use reference iteration

2017-03-31 Thread Michael Haggerty
Use reference iteration rather than do_for_each_entry_in_dir() in the definition of files_pack_refs(). Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 143 +-- 1 file changed, 60 insertions(+), 83 del

[PATCH v2 12/20] ref-cache: use a callback function to fill the cache

2017-03-31 Thread Michael Haggerty
` to supply a pointer to function `read_loose_refs` (renamed to `loose_fill_ref_dir`) when creating the ref cache for its loose refs. This means that we can generify the type of the back-pointer in `struct ref_cache` from `files_ref_store` to `ref_store`. Signed-off-by: Michael Haggerty <m

[PATCH v2 08/20] ref-cache: rename `remove_entry()` to `remove_entry_from_dir()`

2017-03-31 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH v2 16/20] get_loose_ref_cache(): new function

2017-03-31 Thread Michael Haggerty
Extract a new function, `get_loose_ref_cache()`, from get_loose_ref_dir(). The function returns the `ref_cache` for the loose refs of a `files_ref_store`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 9 +++-- 1 file changed, 7 insertions(+), 2 del

[PATCH v2 06/20] ref-cache: rename `add_ref()` to `add_ref_entry()`

2017-03-31 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH v2 00/20] Separate `ref_cache` into a separate module

2017-03-31 Thread Michael Haggerty
efs(): use reference iteration This patch series is also available from my GitHub fork [2] as branch "separate-ref-cache". These patches depend on Duy's nd/files-backend-git-dir branch. [1] http://public-inbox.org/git/cover.1490026594.git.mhag...@alum.mit.edu/ [2] https://gith

[PATCH v2 04/20] refs_verify_refname_available(): implement once for all backends

2017-03-31 Thread Michael Haggerty
It turns out that we can now implement `refs_verify_refname_available()` based on the other virtual functions, so there is no need for it to be defined at the backend level. Instead, define it once in `refs.c` and remove the `files_backend` definition. Signed-off-by: Michael Haggerty <m

[PATCH v2 09/20] refs: split `ref_cache` code into separate files

2017-03-31 Thread Michael Haggerty
, adds declarations, and changes the visibility of some functions, but doesn't change any code. The modules are still too tightly coupled, but the situation will be improved in subsequent commits. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- Makefile | 1 + refs

[PATCH v2 15/20] get_loose_ref_dir(): function renamed from get_loose_refs()

2017-03-31 Thread Michael Haggerty
The new name is more analogous to `get_packed_ref_dir()`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 0ff5df6b46..0a16f6196c

[PATCH v2 02/20] refs_read_raw_ref(): new function

2017-03-31 Thread Michael Haggerty
Extract a new function from `refs_resolve_ref_unsafe()`. It will be useful elsewhere. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 11 +-- refs/refs-internal.h | 4 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/refs.c b/

[PATCH v2 07/20] ref-cache: rename `find_ref()` to `find_ref_entry()`

2017-03-31 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH v2 05/20] refs_verify_refname_available(): use function in more places

2017-03-31 Thread Michael Haggerty
(and will be regained later). These were the last callers of `verify_refname_available_dir()`, so also delete that (very complicated) function. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 171 --- 1 file chang

[PATCH v2 01/20] get_ref_dir(): don't call read_loose_refs() for "refs/bisect"

2017-03-31 Thread Michael Haggerty
nism, and this time the read was done correctly. This code has been broken since it was first introduced. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 42

[PATCH v2 03/20] refs_ref_iterator_begin(): new function

2017-03-31 Thread Michael Haggerty
Extract a new function from `do_for_each_ref()`. It will be useful elsewhere. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 15 +-- refs/refs-internal.h | 11 +++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/re

[PATCH] status: show in-progress info for short status

2017-03-31 Thread Michael J Gruber
information is shown next to the branch information. Just like `--branch`, this comes with a config option. The wording for the in-progress information is taken over from git-prompt.sh. Signed-off-by: Michael J Gruber <g...@grubix.eu> --- When used with --porcelain, this gives an easy way to

[PATCH v3 3/4] name-rev: provide debug output

2017-03-31 Thread Michael J Gruber
cribe" header). The date cut-off for name-rev kicks in way more often than the candidate number cut-off of describe, so we do not clutter the output with the cut-off. Signed-off-by: Michael J Gruber <g...@grubix.eu> --- Documentation/git-name-rev.txt | 5 builtin/name-rev.c

[PATCH v3 4/4] describe: pass --debug down to name-rev

2017-03-31 Thread Michael J Gruber
Now that name-rev knows --debug, pass that flag down to name-rev when we call it for doing describe --contains. Signed-off-by: Michael J Gruber <g...@grubix.eu> --- builtin/describe.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/describe.c b/builtin/describe.c index a5cd

[PATCH v3 2/4] name-rev: favor describing with tags and use committer date to tiebreak

2017-03-31 Thread Michael J Gruber
I do not mind people disagreeing with the logic that this patch happens to implement. This is done primarily to illustrate the value of using a separate helper function is_better_name() instead of open-coding the selection logic in name_rev() function. Signed-off-by: Michael J Gru

[PATCH v3 0/4] name-rev sanity

2017-03-31 Thread Michael J Gruber
describing with tags and use committer date to tiebreak Michael J Gruber (2): name-rev: provide debug output describe: pass --debug down to name-rev Documentation/git-name-rev.txt | 5 ++ builtin/describe.c | 2 + builtin/name-rev.c | 117

[PATCH v3 1/4] name-rev: refactor logic to see if a new candidate is a better name

2017-03-31 Thread Michael J Gruber
while keeping the current logic the same (i.e. a name that is based on an older tag is better, and if two tags of the same age can reach the commit, the one with fewer number of hops to reach the commit is better). Signed-off-by: Junio C Hamano <gits...@pobox.com> Signed-off-by: Michael

Re: [RFC PATCH 0/5] Localise error headers

2017-03-30 Thread Michael J Gruber
> further analysis. You can do that by string-matching "BUG:" from the > beginning of a die message, but it's kind of gross. :) > > -Peff I read back the whole thread, and I'm still not sure if there's consensus and how to go forward. Should we let the topic die? I don't care too much personally, I just thought the mixed tranlations look "unprofessional". Michael

Re: [PATCH v2 0/3] name-rev sanity

2017-03-30 Thread Michael J Gruber
Junio C Hamano venit, vidit, dixit 29.03.2017 19:43: > Junio C Hamano writes: > >> The first two applies cleanly to the same base as jc/name-rev that >> the first two of these patches are meant to replace, but the third >> one doesn't apply on top. Are you depending on

Re: [PATCH v5 0/6] [GSoC] remove_subtree(): reimplement using iterators

2017-03-30 Thread Michael Haggerty
g/git/1490747533-89143-1-git-send-email-bnm...@gmail.com/T/#e437a63e0c22c00c69b5d92977c9b438ed2b9fd3a > > I would like to really thank Michael for the incredibly thorough review of > the last version of this series. I never expected anyone to give that > level of attention to thi

Re: [PATCH v5 6/6] remove_subtree(): test removing nested directories

2017-03-30 Thread Michael Haggerty
; + echo content >path/file1 && > + echo content >path/with/nested/paths/file2 && > + git checkout-index -f -a && > + test ! -d path > +' > + > test_done > It would be better for this patch to precede "remove_subtree(): reimplement using iterators", as a slightly better proof that the change to using iterators doesn't change the behavior. Michael

Re: [PATCH v5 3/6] dir_iterator: iterate over dir after its contents

2017-03-30 Thread Michael Haggerty
} } else { level->dir_state = DIR_STATE_ITERATED; } } else if (level->dir_state == DIR_STATE_ITERATED) { if (iter->flags & DIR_ITERATOR_DIRS_AFTER) { ...set iterator data... level->dir_state = DIR_STATE_POST_ITERATED; return ITER_OK; } else { level->dir_state = DIR_STATE_POST_ITERATED; } } else if (level->dir_state == DIR_STATE_POST_ITERATED) { ...pop dir... if (...done...) { break; } } } It's a biggish rewrite (and I'm not implying that you have to do it), but I think the results would be more readable than the current code. Michael

Re: [PATCH v5 2/6] dir_iterator: refactor state machine model

2017-03-30 Thread Michael Haggerty
r_level > struct and ease comprehension of the state machine's behavior. Nice. Michael

Re: [PATCH v5 4/6] dir_iterator: add tests for dir_iterator API

2017-03-30 Thread Michael Haggerty
; + > + return 0; > +} > [...] Oh I forgot to mention, in the Git project we don't allow declarations to be mixed with code. Apparently there's some ancient compiler somewhere that doesn't allow it. Declarations always have to be together, at the top of a block. (Compile with `-Werror=declaration-after-statement` to detect this.) Michael

Re: [PATCH v5 4/6] dir_iterator: add tests for dir_iterator API

2017-03-30 Thread Michael Haggerty
directory tree and that post-order directory > iteration is correctly implemented. Please also add the compiled test program `test-dir-iterator` to `t/helper/.gitignore`. Michael

Re: [PATCH v5 4/6] dir_iterator: add tests for dir_iterator API

2017-03-30 Thread Michael Haggerty
pre-order and once for post-order traversal. You could use the same directory setup and the same `expected-sorted` file for both tests (e.g., set them up in a preliminary "setup" step). > +' > + > +ITER_POST_ORDER_OUTPUT='[f] (a/b/c/d) ./dir2/a/b/c/d > +[d] (a/b/c) ./dir2/a/b/c > +[d] (a/b) ./dir2/a/b > +[d] (a) ./dir2/a' > + > +test_expect_success 'dir-iterator should list files properly on post-order > mode' ' > + mkdir -p dir2/a/b/c/ && > + date >dir2/a/b/c/d && > + > + test "$(test-dir-iterator ./dir2 --post-order)" == > "$ITER_POST_ORDER_OUTPUT" > +' How about adding a test for pre-order traversal, too? It would look almost identical to this test. > [...] Michael

Re: [PATCH v4 2/5] dir_iterator: iterate over dir after its contents

2017-03-30 Thread Michael Haggerty
On 03/30/2017 08:08 AM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> I think IN_ORDER really only applies to *binary* trees, not arbitrary >> trees like a filesystem. > > How true. Even if we were giving a sorted outp

Re: [PATCH v4 2/5] dir_iterator: iterate over dir after its contents

2017-03-29 Thread Michael Haggerty
On 03/29/2017 06:46 PM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> I also realize that I made a goof in my comments about v3 of this patch >> series. Your new option is not choosing between "depth-first" and >> "brea

[PATCH v2 0/3] name-rev sanity

2017-03-29 Thread Michael J Gruber
, but still mostly Junio's Michael J Gruber (1): name-rev: provide debug output This replaces the patch which documented that --debug does not work with --contains :) builtin/describe.c | 2 + builtin/name-rev.c | 117 + t/t4202-log.sh

[PATCH v2 2/3] name-rev: favor describing with tags and use committer date to tiebreak

2017-03-29 Thread Michael J Gruber
I do not mind people disagreeing with the logic that this patch happens to implement. This is done primarily to illustrate the value of using a separate helper function is_better_name() instead of open-coding the selection logic in name_rev() function. Signed-off-by: Michael J Gru

[PATCH v2 3/3] name-rev: provide debug output

2017-03-29 Thread Michael J Gruber
te cut-off for name-rev kicks in way more often than the candidate number cut-off of describe, so we do not clutter the output with the cut-off. Signed-off-by: Michael J Gruber <g...@grubix.eu> --- builtin/describe.c | 2 ++ builtin/name-rev.c | 64 +++

[PATCH v2 1/3] name-rev: refactor logic to see if a new candidate is a better name

2017-03-29 Thread Michael J Gruber
while keeping the current logic the same (i.e. a name that is based on an older tag is better, and if two tags of the same age can reach the commit, the one with fewer number of hops to reach the commit is better). Signed-off-by: Junio C Hamano <gits...@pobox.com> Signed-off-by: Michael

Re: [PATCH v4 2/5] dir_iterator: iterate over dir after its contents

2017-03-29 Thread Michael Haggerty
On 03/29/2017 11:56 AM, Michael Haggerty wrote: > On 03/29/2017 02:32 AM, Daniel Ferreira wrote: >> [...] > [...] > The disagreement is not a surprise, because there isn't a corresponding > coding error in the code below that returns the directory itself in a > post-order iter

Re: [PATCH v6 18/27] files-backend: replace submodule_allowed check in files_downcast()

2017-03-29 Thread Michael Haggerty
On 03/26/2017 04:16 AM, Duy Nguyen wrote: > On Mon, Mar 20, 2017 at 4:18 AM, Michael Haggerty <mhag...@alum.mit.edu> > wrote: >>> +/* ref_store_init flags */ >>> +#define REF_STORE_READ (1 << 0) >> >> I asked [1] in reply to v5 wheth

Re: [PATCH v4 5/5] files_reflog_iterator: amend use of dir_iterator

2017-03-29 Thread Michael Haggerty
gs")); > + iter->dir_iterator = dir_iterator_begin(git_path("logs"), 0); > return ref_iterator; > } As mentioned earlier, this patch should be squashed into patch [2/5]. Michael

Re: [PATCH v4 3/5] remove_subtree(): reimplement using iterators

2017-03-29 Thread Michael Haggerty
die_errno("cannot unlink '%s'", path->buf); The two `die_errno()` calls must be changed to use `diter->path.buf` rather than `path->buf`. > - strbuf_setlen(path, origlen); > } > - closedir(dir); > + > if (rmdir(path->buf)) > die_errno("cannot rmdir '%s'", path->buf); > } Michael

Re: [PATCH v4 2/5] dir_iterator: iterate over dir after its contents

2017-03-29 Thread Michael Haggerty
if (!path || !*path) > die("BUG: empty path passed to dir_iterator_begin()"); > > + iter->flags = flags; > + > strbuf_init(>base.path, PATH_MAX); > strbuf_addstr(>base.path, path); > > diff --git a/dir-iterator.h b/dir-iterator.h > index 27739e6..28ff3df 100644 > --- a/dir-iterator.h > +++ b/dir-iterator.h > @@ -38,6 +38,13 @@ > * dir_iterator_advance() again. > */ The module docstring just above this hunk needs updating, too. > +/* Possible flags for dir_iterator_begin(). > + * > + * DIR_ITERATOR_DEPTH_FIRST: ensures subdirectories and their contents > + * are iterated through before the containing directory. > + */ > +#define DIR_ITERATOR_DEPTH_FIRST (1 << 1) > + Normally the first constant in a bitset gets the value `(1 << 0)`; i.e., 1. Is there a reason you use 2 here? > [...] Michael

Re: [PATCH v4 1/5] dir_iterator: add helpers to dir_iterator_advance

2017-03-28 Thread Michael Haggerty
tor_int *iter, struct > dir_iterator_level *level) > +{ > + return --iter->levels_nr; > +} `pop_dir_level()` doesn't use its `level` argument; it can be removed. > [...] Michael

[PATCH v2 0/2] describe: localize debug output

2017-03-27 Thread Michael J Gruber
v2 computes the width for the localized output dynamically. In fact, it might overcalculated a bit depending on the encoding, but this does not do any harm. Michael J Gruber (2): describe: localize debug output fully l10n: de: translate describe debug terms builtin/describe.c | 15

[PATCH v2 2/2] l10n: de: translate describe debug terms

2017-03-27 Thread Michael J Gruber
Signed-off-by: Michael J Gruber <g...@grubix.eu> --- po/de.po | 14 +- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/po/de.po b/po/de.po index e9c86f5488..913db393dc 100644 --- a/po/de.po +++ b/po/de.po @@ -7530,7 +7530,19 @@ msgstr "git describe [] [...]&q

[PATCH v2 1/2] describe: localize debug output fully

2017-03-27 Thread Michael J Gruber
git describe --debug localizes all debug messages but not the terms head, lightweight, annotated that it outputs for the candidates. Localize them, too. Signed-off-by: Michael J Gruber <g...@grubix.eu> --- builtin/describe.c | 15 --- 1 file changed, 12 insertions(+), 3 del

Re: [PATCH v3 1/2] [GSoC] dir_iterator: iterate over dir after its contents

2017-03-26 Thread Michael Haggerty
iteration process files before directories within a single directory; rather, it ensures that subdirectories and their contents are processed before the containing directory. Therefore, a better name might be something like "depth_first". I should mention that I like the overall idea to add this new feature and use it to simplify `remove_subtree()`. Michael

Re: [PATCH v3 1/2] [GSoC] dir_iterator: iterate over dir after its contents

2017-03-26 Thread Michael Haggerty
ew caller using the old call style), the compiler would complain, and the problem would be obvious and easy to fix. I didn't actually read the patch carefully yet because I don't have time this evening to seek out the interesting parts in the long diff. Michael > [...]

Re: report on a possible bug: git commit -p myfile.py unexpected output

2017-03-24 Thread Michael J Gruber
ct to see only > deletions in the output message if I am picking only deletions. > > If this is a bug, I hope you can reproduce it on your system. If not, > do not hesitate to contact me for further information. > > > Kind regards, > > Joan Aguilar > -- > Joan Aguilar Lorente > 182-302 = -120 Did you make any changes in the lines that you left? Apparantly, that's what the rewrite looked like to git commit. Michael

Re: [PATCH v3 2/2] l10n: Add git-add.txt to localized man pages

2017-03-24 Thread Michael J Gruber
ng unsure about the consequences), as I would hope that translators would be the same so that we keep consistency across several tranlations in one language. Michael

[PATCH] mailmap: use Michael J Gruber's new address

2017-03-24 Thread Michael J Gruber
Map both old addresses to the new, hopefully more permanent one. Signed-off-by: Michael J Gruber <g...@drmicha.warpmail.net> Signed-off-by: Michael J Gruber <g...@grubix.eu> --- .mailmap | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.mailmap b/.mailmap inde

Re: [PATCH] Documentation/git-worktree: use working tree for trees on the file system

2017-03-23 Thread Michael J Gruber
does: - switching of branches - creation of branches - detaching of head - partial updates of the working tree So why shouldn't it manage worktrees, as well? While that may sound a bit sarcastic it indicates that we may want to rethink some things at some point rather than adding up to the conflation. The discussion in this thread seems to show that "worktree" is just as a good a name for the new feature, while "workbase" or "workroot" (or "workdir") or so could have been for the old one. Are we at a point where we can still rename the new feature at least? If yes, and keeping everything else is mandatory, than "workspace" or "working space" may be a serious contender for naming the new thing. Michael

Re: [PATCH 18/20] commit_packed_refs(): use reference iteration

2017-03-22 Thread Michael Haggerty
On 03/20/2017 07:05 PM, Jeff King wrote: > On Mon, Mar 20, 2017 at 05:33:23PM +0100, Michael Haggerty wrote: > >> -/* >> - * An each_ref_entry_fn that writes the entry to a packed-refs file. >> - */ >> -static int write_packed_entry_fn(struct ref_entry *entry, voi

Re: Use base32?

2017-03-20 Thread Michael Steuer
On 20/03/2017 19:05, Jacob Keller wrote: On Sun, Mar 19, 2017 at 10:58 PM, Michael Steuer <michael.ste...@constrainttec.com> wrote: [..] If base32 is being considered, I'd suggest the "base32hex" variant, which uses the same amount of space. I don't see the benefit of adding c

Re: [PATCH 00/20] Separate `ref_cache` into a separate module

2017-03-20 Thread Michael Haggerty
On 03/20/2017 11:32 PM, Junio C Hamano wrote: > Michael Haggerty <mhag...@alum.mit.edu> writes: > >> Michael Haggerty (20): >> get_ref_dir(): don't call read_loose_refs() for "refs/bisect" >> refs_read_raw_ref(): new function >

Re: [PATCH 11/20] refs: record the ref_store in ref_cache, not ref_dir

2017-03-20 Thread Michael Haggerty
On 03/20/2017 06:51 PM, Jeff King wrote: > On Mon, Mar 20, 2017 at 05:33:16PM +0100, Michael Haggerty wrote: > >> Instead of keeping a pointer to the ref_store in every ref_dir entry, >> store it once in `struct ref_cache`, and change `struct ref_dir` to >> include a p

Re: [PATCH 04/20] refs_verify_refname_available(): implement once for all backends

2017-03-20 Thread Michael Haggerty
On 03/20/2017 06:42 PM, Jeff King wrote: > On Mon, Mar 20, 2017 at 05:33:09PM +0100, Michael Haggerty wrote: > >> It turns out that we can now implement >> `refs_verify_refname_available()` based on the other virtual >> functions, so there is no need for it to be defined

[PATCH 06/20] Rename `add_ref()` to `add_ref_entry()`

2017-03-20 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH 02/20] refs_read_raw_ref(): new function

2017-03-20 Thread Michael Haggerty
Extract a new function from `refs_resolve_ref_unsafe()`. It will be useful elsewhere. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 11 +-- refs/refs-internal.h | 4 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/refs.c b/

[PATCH 04/20] refs_verify_refname_available(): implement once for all backends

2017-03-20 Thread Michael Haggerty
It turns out that we can now implement `refs_verify_refname_available()` based on the other virtual functions, so there is no need for it to be defined at the backend level. Instead, define it once in `refs.c` and remove the `files_backend` definition. Signed-off-by: Michael Haggerty <m

[PATCH 05/20] refs_verify_refname_available(): use function in more places

2017-03-20 Thread Michael Haggerty
(and will be regained later). These were the last callers of `verify_refname_available_dir()`, so also delete that (very complicated) function. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 171 --- 1 file chang

[PATCH 03/20] refs_ref_iterator_begin(): new function

2017-03-20 Thread Michael Haggerty
Extract a new function from `do_for_each_ref()`. It will be useful elsewhere. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs.c | 15 +-- refs/refs-internal.h | 11 +++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/re

[PATCH 17/20] cache_ref_iterator_begin(): make function smarter

2017-03-20 Thread Michael Haggerty
ator_begin()` to be made more ignorant of the internals of `ref_cache`, and `find_containing_dir()` and `prime_ref_dir()` to be made private. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 44 +--- refs/ref-

[PATCH 09/20] refs: split `ref_cache` code into separate files

2017-03-20 Thread Michael Haggerty
, adds declarations, and changes the visibility of some functions, but doesn't change any code. The modules are still too tightly coupled, but the situation will be improved in subsequent commits. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- Makefile | 1 + refs

[PATCH 12/20] ref-cache: use a callback function to fill the cache

2017-03-20 Thread Michael Haggerty
` to supply a pointer to function `read_loose_refs` (renamed to `loose_fill_ref_dir`) when creating the ref cache for its loose refs. This means that we can generify the type of the back-pointer in `struct ref_cache` from `files_ref_store` to `ref_store`. Signed-off-by: Michael Haggerty <m

[PATCH 07/20] Rename `find_ref()` to `find_ref_entry()`

2017-03-20 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH 14/20] do_for_each_entry_in_dir(): eliminate `offset` argument

2017-03-20 Thread Michael Haggerty
It was never used. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 4 ++-- refs/ref-cache.c | 6 +++--- refs/ref-cache.h | 11 +-- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/refs/files-backend.c b/refs/files-bac

[PATCH 19/20] files_pack_refs(): use reference iteration

2017-03-20 Thread Michael Haggerty
Use reference iteration rather than do_for_each_entry_in_dir() in the definition of files_pack_refs(). Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 143 +-- 1 file changed, 60 insertions(+), 83 del

[PATCH 15/20] get_loose_ref_dir(): function renamed from get_loose_refs()

2017-03-20 Thread Michael Haggerty
The new name is more analogous to `get_packed_ref_dir()`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 0ff5df6b46..0a16f6196c

[PATCH 18/20] commit_packed_refs(): use reference iteration

2017-03-20 Thread Michael Haggerty
Use reference iteration rather than do_for_each_entry_in_dir() in the definition of commit_packed_refs(). Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 38 +- 1 file changed, 17 insertions(+), 21 deletions(-) diff

[PATCH 08/20] Rename `remove_entry()` to `remove_entry_from_dir()`

2017-03-20 Thread Michael Haggerty
This function's visibility is about to be increased, so give it a more distinctive name. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c

[PATCH 16/20] get_loose_ref_cache(): new function

2017-03-20 Thread Michael Haggerty
Extract a new function, `get_loose_ref_cache()`, from get_loose_ref_dir(). The function returns the `ref_cache` for the loose refs of a `files_ref_store`. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 9 +++-- 1 file changed, 7 insertions(+), 2 del

[PATCH 20/20] do_for_each_entry_in_dir(): delete function

2017-03-20 Thread Michael Haggerty
Its only remaining caller was itself. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/ref-cache.c | 21 - refs/ref-cache.h | 11 --- 2 files changed, 32 deletions(-) diff --git a/refs/ref-cache.c b/refs/ref-cache.c index b3a30350d7..6059362f1d

[PATCH 11/20] refs: record the ref_store in ref_cache, not ref_dir

2017-03-20 Thread Michael Haggerty
the size of the individual entries. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 6 +++--- refs/ref-cache.c | 12 +++- refs/ref-cache.h | 9 ++--- 3 files changed, 16 insertions(+), 11 deletions(-) diff --git a/refs/files-backend.c

[PATCH 10/20] ref-cache: introduce a new type, ref_cache

2017-03-20 Thread Michael Haggerty
For now, it just wraps a `ref_entry *` that points at the root of the tree. Soon it will hold more information. Add two new functions, `create_ref_cache()` and `free_ref_cache()`. Make `free_ref_entry()` private. Change files-backend to use this type to hold its caches. Signed-off-by: Michael

[PATCH 01/20] get_ref_dir(): don't call read_loose_refs() for "refs/bisect"

2017-03-20 Thread Michael Haggerty
nism, and this time the read was done correctly. This code has been broken since it was first introduced. Signed-off-by: Michael Haggerty <mhag...@alum.mit.edu> --- refs/files-backend.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 42

[PATCH 13/20] refs: handle "refs/bisect/" in `loose_fill_ref_dir()`

2017-03-20 Thread Michael Haggerty
That "refs/bisect/" has to be handled specially when filling the ref_cache for loose references is a peculiarity of the files backend, and the ref-cache code shouldn't need to know about it. So move this code to the callback function, `loose_fill_ref_dir()`. Signed-off-by: Michael Hagg

<    2   3   4   5   6   7   8   9   10   11   >