Re: [PATCH v3 25/25] sequencer: mark all error messages for translation

2016-10-13 Thread Johannes Sixt 

Am 13.10.2016 um 16:56 schrieb Johannes Schindelin:

On Wed, 12 Oct 2016, Junio C Hamano wrote:

You have at least two independent changes relative to Dscho's
version.

 (1) Show line breaks more prominently by avoiding "\n\n" and
 breaking the string at "\n"; this matches the way how the
 result would be displayed more closely to how the source looks
 like.

 (2) Ignore the usual indentation rule and have messages start at
 the left end of the source.

Which one are you saying "makes sense" to?  Both?

I guess both can be grouped together into one theme: match the way
the final output and the source code look like.


Yes, that summarizes it pretty well.


I agree with that motivation, but I decided to go about it in a way that
is more in line with the existing source code:

-- snipsnap --
diff --git a/sequencer.c b/sequencer.c
index 8e10bb5..1cf70f7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -483,6 +483,20 @@ static char **read_author_script(void)
return env;
 }

+static const char staged_changes_advice[] =
+N_("you have staged changes in your working tree\n"
+"If these changes are meant to be squashed into the previous commit, run:\n"
+"\n"
+"  git commit --amend %s\n"
+"\n"
+"If they are meant to go into a new commit, run:\n"
+"\n"
+"  git commit %s\n"
+"\n"
+"In both cases, once you're done, continue with:\n"
+"\n"
+"  git rebase --continue\n");


Much better! Thank you.

-- Hannes

Re: [PATCH v3 25/25] sequencer: mark all error messages for translation

2016-10-13 Thread Johannes Schindelin 
Hi,

On Wed, 12 Oct 2016, Junio C Hamano wrote:

> Johannes Sixt  writes:
> 
> > Can we please have the following change instead? I think it makes sense
> > to deviate from the usual conventions in a case like this.
> 
> You have at least two independent changes relative to Dscho's
> version.  
> 
>  (1) Show line breaks more prominently by avoiding "\n\n" and
>  breaking the string at "\n"; this matches the way how the
>  result would be displayed more closely to how the source looks
>  like.
> 
>  (2) Ignore the usual indentation rule and have messages start at
>  the left end of the source.
> 
> Which one are you saying "makes sense" to?  Both?
> 
> I guess both can be grouped together into one theme: match the way
> the final output and the source code look like.
> 
> If that is the motivation behind "makes sense", I'd prefer to see
> the change explained explicitly with that rationale in the log
> message.
> 
> Thanks.  I personally agree with that motivation (if the one I
> guessed above is your motivation, that is).

I agree with that motivation, but I decided to go about it in a way that
is more in line with the existing source code:

-- snipsnap --
diff --git a/sequencer.c b/sequencer.c
index 8e10bb5..1cf70f7 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -483,6 +483,20 @@ static char **read_author_script(void)
return env;
 }
 
+static const char staged_changes_advice[] =
+N_("you have staged changes in your working tree\n"
+"If these changes are meant to be squashed into the previous commit, run:\n"
+"\n"
+"  git commit --amend %s\n"
+"\n"
+"If they are meant to go into a new commit, run:\n"
+"\n"
+"  git commit %s\n"
+"\n"
+"In both cases, once you're done, continue with:\n"
+"\n"
+"  git rebase --continue\n");
+
 /*
  * If we are cherry-pick, and if the merge did not result in
  * hand-editing, we will hit this commit and inherit the original
@@ -509,18 +523,7 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
if (!env) {
const char *gpg_opt = gpg_sign_opt_quoted(opts);
 
-   return error(_("you have staged changes in your "
-  "working tree. If these changes are "
-  "meant to be\n"
-  "squashed into the previous commit, "
-  "run:\n\n"
-  "  git commit --amend %s\n\n"
-  "If they are meant to go into a new "
-  "commit, run:\n\n"
-  "  git commit %s\n\n"
-  "In both cases, once you're done, "
-  "continue with:\n\n"
-  "  git rebase --continue\n"),
+   return error(_(staged_changes_advice),
 gpg_opt, gpg_opt);
}
}

Re: [PATCH v3 25/25] sequencer: mark all error messages for translation

2016-10-12 Thread Junio C Hamano 
Johannes Sixt  writes:

> Can we please have the following change instead? I think it makes sense
> to deviate from the usual conventions in a case like this.

You have at least two independent changes relative to Dscho's
version.  

 (1) Show line breaks more prominently by avoiding "\n\n" and
 breaking the string at "\n"; this matches the way how the
 result would be displayed more closely to how the source looks
 like.

 (2) Ignore the usual indentation rule and have messages start at
 the left end of the source.

Which one are you saying "makes sense" to?  Both?

I guess both can be grouped together into one theme: match the way
the final output and the source code look like.

If that is the motivation behind "makes sense", I'd prefer to see
the change explained explicitly with that rationale in the log
message.

Thanks.  I personally agree with that motivation (if the one I
guessed above is your motivation, that is).

> Note that this is an error() text, hence, there should not be a
> fullstop on the first line. That's now a good excuse to start the next
> sentence on a new line; hence, this is not a faithful conversion to _()
> anymore (a will happily take authorship and all blame if you don't
> want to for this reason). Also note that _( is not moved to the
> beginning of the line because it would be picked up as hunk header by
> git diff.
>
>  8< 
> From: Johannes Schindelin 
> Subject: [PATCH] sequencer: mark all error messages for translation
>
> There was actually only one error message that was not yet marked for
> translation.
>
> Signed-off-by: Johannes Schindelin 
> Signed-off-by: Johannes Sixt 
> ---
>  sequencer.c | 24 ++--
>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 95a382e..79f7aa4 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -515,16 +515,20 @@ static int run_git_commit(const char *defmsg, struct 
> replay_opts *opts,
>   if (!env) {
>   const char *gpg_opt = gpg_sign_opt_quoted(opts);
>  
> - return error("you have staged changes in your working "
> - "tree. If these changes are meant to be\n"
> - "squashed into the previous commit, run:\n\n"
> - "  git commit --amend %s\n\n"
> - "If they are meant to go into a new commit, "
> - "run:\n\n"
> - "  git commit %s\n\n"
> - "In both cases, once you're done, continue "
> - "with:\n\n"
> - "  git rebase --continue\n", gpg_opt, gpg_opt);
> + return error(_(
> +"you have staged changes in your working tree\n"
> +"If these changes are meant to be squashed into the previous commit, run:\n"
> +"\n"
> +"  git commit --amend %s\n"
> +"\n"
> +"If they are meant to go into a new commit, run:\n"
> +"\n"
> +"  git commit %s\n"
> +"\n"
> +"In both cases, once you're done, continue with:\n"
> +"\n"
> +"  git rebase --continue\n"),
> +  gpg_opt, gpg_opt);
>   }
>   }

Re: [PATCH v3 25/25] sequencer: mark all error messages for translation

2016-10-12 Thread Johannes Sixt 
Am 10.10.2016 um 19:26 schrieb Johannes Schindelin:
> There was actually only one error message that was not yet marked for
> translation.
> 
> Signed-off-by: Johannes Schindelin 
> ---
>  sequencer.c | 23 +--
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 676f16c..86d86ce 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -515,16 +515,19 @@ static int run_git_commit(const char *defmsg, struct 
> replay_opts *opts,
>   if (!env) {
>   const char *gpg_opt = gpg_sign_opt_quoted(opts);
>  
> - return error("you have staged changes in your working "
> - "tree. If these changes are meant to be\n"
> - "squashed into the previous commit, run:\n\n"
> - "  git commit --amend %s\n\n"
> - "If they are meant to go into a new commit, "
> - "run:\n\n"
> - "  git commit %s\n\n"
> - "In both cases, once you're done, continue "
> - "with:\n\n"
> - "  git rebase --continue\n", gpg_opt, gpg_opt);
> + return error(_("you have staged changes in your "
> +"working tree. If these changes are "
> +"meant to be\n"
> +"squashed into the previous commit, "
> +"run:\n\n"
> +"  git commit --amend %s\n\n"
> +"If they are meant to go into a new "
> +"commit, run:\n\n"
> +"  git commit %s\n\n"
> +"In both cases, once you're done, "
> +"continue with:\n\n"
> +"  git rebase --continue\n"),
> +  gpg_opt, gpg_opt);
>   }
>   }
>  
> 

Can we please have the following change instead? I think it makes sense
to deviate from the usual conventions in a case like this.

Note that this is an error() text, hence, there should not be a
fullstop on the first line. That's now a good excuse to start the next
sentence on a new line; hence, this is not a faithful conversion to _()
anymore (a will happily take authorship and all blame if you don't
want to for this reason). Also note that _( is not moved to the
beginning of the line because it would be picked up as hunk header by
git diff.

 8< 
From: Johannes Schindelin 
Subject: [PATCH] sequencer: mark all error messages for translation

There was actually only one error message that was not yet marked for
translation.

Signed-off-by: Johannes Schindelin 
Signed-off-by: Johannes Sixt 
---
 sequencer.c | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 95a382e..79f7aa4 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -515,16 +515,20 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
if (!env) {
const char *gpg_opt = gpg_sign_opt_quoted(opts);
 
-   return error("you have staged changes in your working "
-   "tree. If these changes are meant to be\n"
-   "squashed into the previous commit, run:\n\n"
-   "  git commit --amend %s\n\n"
-   "If they are meant to go into a new commit, "
-   "run:\n\n"
-   "  git commit %s\n\n"
-   "In both cases, once you're done, continue "
-   "with:\n\n"
-   "  git rebase --continue\n", gpg_opt, gpg_opt);
+   return error(_(
+"you have staged changes in your working tree\n"
+"If these changes are meant to be squashed into the previous commit, run:\n"
+"\n"
+"  git commit --amend %s\n"
+"\n"
+"If they are meant to go into a new commit, run:\n"
+"\n"
+"  git commit %s\n"
+"\n"
+"In both cases, once you're done, continue with:\n"
+"\n"
+"  git rebase --continue\n"),
+gpg_opt, gpg_opt);
}
}
 
-- 
2.10.0.343.g37bc62b

[PATCH v3 25/25] sequencer: mark all error messages for translation

2016-10-10 Thread Johannes Schindelin 
There was actually only one error message that was not yet marked for
translation.

Signed-off-by: Johannes Schindelin 
---
 sequencer.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 676f16c..86d86ce 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -515,16 +515,19 @@ static int run_git_commit(const char *defmsg, struct 
replay_opts *opts,
if (!env) {
const char *gpg_opt = gpg_sign_opt_quoted(opts);
 
-   return error("you have staged changes in your working "
-   "tree. If these changes are meant to be\n"
-   "squashed into the previous commit, run:\n\n"
-   "  git commit --amend %s\n\n"
-   "If they are meant to go into a new commit, "
-   "run:\n\n"
-   "  git commit %s\n\n"
-   "In both cases, once you're done, continue "
-   "with:\n\n"
-   "  git rebase --continue\n", gpg_opt, gpg_opt);
+   return error(_("you have staged changes in your "
+  "working tree. If these changes are "
+  "meant to be\n"
+  "squashed into the previous commit, "
+  "run:\n\n"
+  "  git commit --amend %s\n\n"
+  "If they are meant to go into a new "
+  "commit, run:\n\n"
+  "  git commit %s\n\n"
+  "In both cases, once you're done, "
+  "continue with:\n\n"
+  "  git rebase --continue\n"),
+gpg_opt, gpg_opt);
}
}
 
-- 
2.10.0.windows.1.325.ge6089c1