Re: [PATCH v2 00/10] rerere: handle nested conflicts
On 07/06, Junio C Hamano wrote: > Thomas Gummerer writes: > > > On 06/05, Thomas Gummerer wrote: > >> The previous round was at > >> <20180520211210.1248-1-t.gumme...@gmail.com>. > >> > >> Thanks Junio for the comments on the previous round. > >> > >> Changes since v2: > >> - lowercase the first letter in some error/warning messages before > >>marking them for translation > >> - wrap paths in output in single quotes, for consistency, and to make > >>some of the messages the same as ones that are already translated > >> - mark messages in builtin/rerere.c for translation as well, which I > >>had previously forgotten. > >> - expanded the technical documentation on rerere. The entire > >>document is basically rewritten. > >> - changed the test in 6/10 to just fake a conflict marker inside of > >>one of the hunks instead of using an inner conflict created by a > >>merge. This is to make sure the codepath is still hit after we > >>handle inner conflicts properly. > >> - added tests for handling inner conflict markers > >> - added one commit to recalculate the conflict ID when an unresolved > >>conflict is committed, and the subsequent operation conflicts again > >>in the same file. More explanation in the commit message of that > >>commit. > > > > Now that 2.18 is out (and I'm caught up on the list after being away > > from it for a few days), is there any interest in this series? I guess > > it was overlooked as it's been sent in the rc phase for 2.18. > > I deliberately ignored, not because I wasn't interested in it, but > because I'd be distracted during the pre-release feature freeze as > I'd be heavily intereseted in it. > > Now is a good time to repost to stir/re-ignite the interest from > others, possibly after rebasing on v2.18.0 and polishing further. I sometimes find it hard to gauge whether there are no replies because nobody is interested in the series, or if it is because it was ignored or slipped to the cracks. I guess I could have inferred it from your replies to the previous iteration though :) I'll go back and polish my patches, and then send a new iteration, thanks! > Thanks. > > > > > I think the most important bit here is 6/10 which fixes a crash that > > can happen in "normal" usage of git. The translation bits are also > > nice to have I think, but I could send them in a different series if > > that's preferred. > > > > The other patches would be nice to have, but are arguably less > > important. > > > >> range-diff below. A few commits changed enough for range-diff > >> to give up showing the differences in those, they are probably best > >> reviewed as the whole patch anyway: > >> > >> [snip] > >> > >> Thomas Gummerer (10): > >> rerere: unify error messages when read_cache fails > >> rerere: lowercase error messages > >> rerere: wrap paths in output in sq > >> rerere: mark strings for translation > >> rerere: add some documentation > >> rerere: fix crash when conflict goes unresolved > >> rerere: only return whether a path has conflicts or not > >> rerere: factor out handle_conflict function > >> rerere: teach rerere to handle nested conflicts > >> rerere: recalculate conflict ID when unresolved conflict is committed > >> > >> Documentation/technical/rerere.txt | 182 + > >> builtin/rerere.c | 4 +- > >> rerere.c | 246 ++--- > >> t/t4200-rerere.sh | 67 > >> 4 files changed, 372 insertions(+), 127 deletions(-) > >> create mode 100644 Documentation/technical/rerere.txt > >> > >> -- > >> 2.18.0.rc1.242.g61856ae69 > >>
Re: [PATCH v2 00/10] rerere: handle nested conflicts
Thomas Gummerer writes: > On 06/05, Thomas Gummerer wrote: >> The previous round was at >> <20180520211210.1248-1-t.gumme...@gmail.com>. >> >> Thanks Junio for the comments on the previous round. >> >> Changes since v2: >> - lowercase the first letter in some error/warning messages before >>marking them for translation >> - wrap paths in output in single quotes, for consistency, and to make >>some of the messages the same as ones that are already translated >> - mark messages in builtin/rerere.c for translation as well, which I >>had previously forgotten. >> - expanded the technical documentation on rerere. The entire >>document is basically rewritten. >> - changed the test in 6/10 to just fake a conflict marker inside of >>one of the hunks instead of using an inner conflict created by a >>merge. This is to make sure the codepath is still hit after we >>handle inner conflicts properly. >> - added tests for handling inner conflict markers >> - added one commit to recalculate the conflict ID when an unresolved >>conflict is committed, and the subsequent operation conflicts again >>in the same file. More explanation in the commit message of that >>commit. > > Now that 2.18 is out (and I'm caught up on the list after being away > from it for a few days), is there any interest in this series? I guess > it was overlooked as it's been sent in the rc phase for 2.18. I deliberately ignored, not because I wasn't interested in it, but because I'd be distracted during the pre-release feature freeze as I'd be heavily intereseted in it. Now is a good time to repost to stir/re-ignite the interest from others, possibly after rebasing on v2.18.0 and polishing further. Thanks. > > I think the most important bit here is 6/10 which fixes a crash that > can happen in "normal" usage of git. The translation bits are also > nice to have I think, but I could send them in a different series if > that's preferred. > > The other patches would be nice to have, but are arguably less > important. > >> range-diff below. A few commits changed enough for range-diff >> to give up showing the differences in those, they are probably best >> reviewed as the whole patch anyway: >> >> [snip] >> >> Thomas Gummerer (10): >> rerere: unify error messages when read_cache fails >> rerere: lowercase error messages >> rerere: wrap paths in output in sq >> rerere: mark strings for translation >> rerere: add some documentation >> rerere: fix crash when conflict goes unresolved >> rerere: only return whether a path has conflicts or not >> rerere: factor out handle_conflict function >> rerere: teach rerere to handle nested conflicts >> rerere: recalculate conflict ID when unresolved conflict is committed >> >> Documentation/technical/rerere.txt | 182 + >> builtin/rerere.c | 4 +- >> rerere.c | 246 ++--- >> t/t4200-rerere.sh | 67 >> 4 files changed, 372 insertions(+), 127 deletions(-) >> create mode 100644 Documentation/technical/rerere.txt >> >> -- >> 2.18.0.rc1.242.g61856ae69 >>
Re: [PATCH v2 00/10] rerere: handle nested conflicts
On 06/05, Thomas Gummerer wrote: > The previous round was at > <20180520211210.1248-1-t.gumme...@gmail.com>. > > Thanks Junio for the comments on the previous round. > > Changes since v2: > - lowercase the first letter in some error/warning messages before >marking them for translation > - wrap paths in output in single quotes, for consistency, and to make >some of the messages the same as ones that are already translated > - mark messages in builtin/rerere.c for translation as well, which I >had previously forgotten. > - expanded the technical documentation on rerere. The entire >document is basically rewritten. > - changed the test in 6/10 to just fake a conflict marker inside of >one of the hunks instead of using an inner conflict created by a >merge. This is to make sure the codepath is still hit after we >handle inner conflicts properly. > - added tests for handling inner conflict markers > - added one commit to recalculate the conflict ID when an unresolved >conflict is committed, and the subsequent operation conflicts again >in the same file. More explanation in the commit message of that >commit. Now that 2.18 is out (and I'm caught up on the list after being away from it for a few days), is there any interest in this series? I guess it was overlooked as it's been sent in the rc phase for 2.18. I think the most important bit here is 6/10 which fixes a crash that can happen in "normal" usage of git. The translation bits are also nice to have I think, but I could send them in a different series if that's preferred. The other patches would be nice to have, but are arguably less important. > range-diff below. A few commits changed enough for range-diff > to give up showing the differences in those, they are probably best > reviewed as the whole patch anyway: > > [snip] > > Thomas Gummerer (10): > rerere: unify error messages when read_cache fails > rerere: lowercase error messages > rerere: wrap paths in output in sq > rerere: mark strings for translation > rerere: add some documentation > rerere: fix crash when conflict goes unresolved > rerere: only return whether a path has conflicts or not > rerere: factor out handle_conflict function > rerere: teach rerere to handle nested conflicts > rerere: recalculate conflict ID when unresolved conflict is committed > > Documentation/technical/rerere.txt | 182 + > builtin/rerere.c | 4 +- > rerere.c | 246 ++--- > t/t4200-rerere.sh | 67 > 4 files changed, 372 insertions(+), 127 deletions(-) > create mode 100644 Documentation/technical/rerere.txt > > -- > 2.18.0.rc1.242.g61856ae69 >
[PATCH v2 00/10] rerere: handle nested conflicts
The previous round was at
<20180520211210.1248-1-t.gumme...@gmail.com>.
Thanks Junio for the comments on the previous round.
Changes since v2:
- lowercase the first letter in some error/warning messages before
marking them for translation
- wrap paths in output in single quotes, for consistency, and to make
some of the messages the same as ones that are already translated
- mark messages in builtin/rerere.c for translation as well, which I
had previously forgotten.
- expanded the technical documentation on rerere. The entire
document is basically rewritten.
- changed the test in 6/10 to just fake a conflict marker inside of
one of the hunks instead of using an inner conflict created by a
merge. This is to make sure the codepath is still hit after we
handle inner conflicts properly.
- added tests for handling inner conflict markers
- added one commit to recalculate the conflict ID when an unresolved
conflict is committed, and the subsequent operation conflicts again
in the same file. More explanation in the commit message of that
commit.
range-diff below. A few commits changed enough for range-diff
to give up showing the differences in those, they are probably best
reviewed as the whole patch anyway:
1: 901b638400 ! 1: 2825342cc2 rerere: unify error message when read_cache
fails
@@ -1,6 +1,6 @@
Author: Thomas Gummerer
-rerere: unify error message when read_cache fails
+rerere: unify error messages when read_cache fails
We have multiple different variants of the error message we show to
the user if 'read_cache' fails. The "Could not read index" variant we
-: -- > 2: d1500028aa rerere: lowercase error messages
-: -- > 3: ed3601ee71 rerere: wrap paths in output in sq
2: c48ffededd ! 4: 6ead84a199 rerere: mark strings for translation
@@ -9,6 +9,28 @@
Signed-off-by: Thomas Gummerer
+diff --git a/builtin/rerere.c b/builtin/rerere.c
+--- a/builtin/rerere.c
b/builtin/rerere.c
+@@
+ if (!strcmp(argv[0], "forget")) {
+ struct pathspec pathspec;
+ if (argc < 2)
+- warning("'git rerere forget' without paths is
deprecated");
++ warning(_("'git rerere forget' without paths is
deprecated"));
+ parse_pathspec(, 0, PATHSPEC_PREFER_CWD,
+ prefix, argv + 1);
+ return rerere_forget();
+@@
+ const char *path = merge_rr.items[i].string;
+ const struct rerere_id *id = merge_rr.items[i].util;
+ if (diff_two(rerere_path(id, "preimage"), path, path,
path))
+- die("unable to generate diff for '%s'",
rerere_path(id, NULL));
++ die(_("unable to generate diff for '%s'"),
rerere_path(id, NULL));
+ }
+ } else
+ usage_with_options(rerere_usage, options);
+
diff --git a/rerere.c b/rerere.c
--- a/rerere.c
+++ b/rerere.c
@@ -53,14 +75,14 @@
io.input = fopen(path, "r");
io.io.wrerror = 0;
if (!io.input)
-- return error_errno("Could not open %s", path);
-+ return error_errno(_("Could not open %s"), path);
+- return error_errno("could not open '%s'", path);
++ return error_errno(_("could not open '%s'"), path);
if (output) {
io.io.output = fopen(output, "w");
if (!io.io.output) {
-- error_errno("Could not write %s", output);
-+ error_errno(_("Could not write %s"), output);
+- error_errno("could not write '%s'", output);
++ error_errno(_("could not write '%s'"), output);
fclose(io.input);
return -1;
}
@@ -68,18 +90,18 @@
fclose(io.input);
if (io.io.wrerror)
-- error("There were errors while writing %s (%s)",
-+ error(_("There were errors while writing %s (%s)"),
+- error("there were errors while writing '%s' (%s)",
++ error(_("there were errors while writing '%s' (%s)"),
path, strerror(io.io.wrerror));
if (io.io.output && fclose(io.io.output))
-- io.io.wrerror = error_errno("Failed to flush %s", path);
-+ io.io.wrerror = error_errno(_("Failed to flush %s"), path);
+- io.io.wrerror = error_errno("failed to flush '%s'", path);
++ io.io.wrerror = error_errno(_("failed to flush '%s'"), path);
if (hunk_no < 0) {
if (output)
unlink_or_warn(output);
-- return error("Could not parse conflict hunks in %s", path);
-+ return error(_("Could not parse conflict hunks
