You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Karl Fogel <kf...@red-bean.com> on 2008/06/05 22:17:17 UTC
Re: [PATCH] proper indentation in merge conflict menu
Whoa, whoa, whoa. Let's slow down and think about this carefully.
I'm not sure r31600 is a generally applicable solution. After all, a
translation might be longer instead of shorter.
Instead, perhaps we have to write multiline prompts in a way that allows
translations to work regardless of word length. I don't think counting
characters is the way to go, unless there's no other solution.
Do you know of any other places in Subversion that have this problem?
-Karl
Jens Seidel <je...@users.sf.net> writes:
> OK, there is indeed similar code in trunk and I prepared a patch which
> fixes the indentation (but should not be backported so r31600 is fine) from:
>
> Auswahl: (p) zurückstellen, (df) voller Diff, (e) editieren,
> (mc) mine-conflict, (tc) theirs-conflict,
> (s) show all options:
>
> to
>
> Auswahl: (p) zurückstellen, (df) voller Diff, (e) editieren,
> (mc) mine-conflict, (tc) theirs-conflict,
> (s) show all options:
>
> Nevertheless I need help as I'm really not comfortable with apr. I did
> not even find good documentation (only
> http://apr.apache.org/docs/apr/1.3, nothing about pool usage ...).
>
> How can I count the number of characters in a string? strlen would
> return a too large value because of multi-byte characters.
>
> See the attached patch for details.
>
> [[[
> Fixed indentation of the translated merge conflict resolution menu.
>
> * subversion/svn/conflict-callbacks.c
> (svn_cl__conflict_handler):
> The code didn't respect that translations of "Select: " could have another
> length than 8 (the English one). That's why further shortcuts were wrongly
> aligned.
> ]]]
>
> Index: subversion/svn/conflict-callbacks.c
> ===================================================================
> --- subversion/svn/conflict-callbacks.c (Revision 31602)
> +++ subversion/svn/conflict-callbacks.c (Arbeitskopie)
> @@ -427,8 +427,16 @@
> {
> svn_pool_clear(subpool);
>
> - prompt = apr_pstrdup(subpool, _("Select: (p) postpone"));
> + char *indentation = apr_pstrdup(subpool, "");
> + int i;
> + // FIXME: strlen() is too large for multi-byte streams! How to
> + // count characters (not bytes)?
> + for (i=0; i<strlen(_("Select: ")); ++i)
> + indentation = apr_pstrcat(subpool, indentation, " ", NULL);
>
> + prompt = apr_pstrdup(subpool, _("Select: "));
> + prompt = apr_pstrcat(subpool, prompt, _("(p) postpone"), NULL);
> +
> if (diff_allowed)
> {
> prompt = apr_pstrcat(subpool, prompt,
> @@ -441,23 +449,29 @@
>
> if (! desc->is_binary &&
> desc->kind != svn_wc_conflict_kind_property)
> - prompt = apr_pstrcat(subpool, prompt,
> - _(",\n (mc) mine-conflict, "
> - "(tc) theirs-conflict"),
> - NULL);
> + {
> + prompt = apr_pstrcat(subpool, prompt, _(",\n"), NULL);
> + prompt = apr_pstrcat(subpool, prompt, indentation, NULL);
> + prompt = apr_pstrcat(subpool, prompt,
> + _("(mc) mine-conflict, "
> + "(tc) theirs-conflict"),
> + NULL);
> + }
> }
> else
> {
> if (knows_something)
> prompt = apr_pstrcat(subpool, prompt, _(", (r) resolved"),
> NULL);
> + prompt = apr_pstrcat(subpool, prompt, _(",\n"), NULL);
> + prompt = apr_pstrcat(subpool, prompt, indentation, NULL);
> prompt = apr_pstrcat(subpool, prompt,
> - _(",\n "
> - "(mf) mine-full, (tf) theirs-full"),
> + _("(mf) mine-full, (tf) theirs-full"),
> NULL);
> }
>
> - prompt = apr_pstrcat(subpool, prompt, ",\n ", NULL);
> + prompt = apr_pstrcat(subpool, prompt, _(",\n"), NULL);
> + prompt = apr_pstrcat(subpool, prompt, indentation, NULL);
> prompt = apr_pstrcat(subpool, prompt,
> _("(s) show all options: "),
> NULL);
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] proper indentation in merge conflict menu
Posted by Jens Seidel <je...@users.sourceforge.net>.
On Fri, Jun 06, 2008 at 04:37:39PM +0200, Stefan Sperling wrote:
> On Fri, Jun 06, 2008 at 03:56:57PM +0200, Jens Seidel wrote:
> > On Fri, Jun 06, 2008 at 03:06:42PM +0200, Stefan Sperling wrote:
> > > Note that probably not all systems we support have wide chars, so on
> >
> > I think one such system is cygwin.
> >
> > > What about maintaining multiple full text menus? The code could use
> >
> > I do not like text duplication. I have to translate it consistently.
>
> We're currently talking of about 5 or 6 full text prompts that will share
> most of their text. Each being about 3 lines long. I think this is a
> negligible amount of additional translation overhead, considering that
> it buys us consistent indentation for all languages on any platform
> we support. And it keeps the code's complexity at the same level as it
> currently is.
Mmh, OK. Nevertheless it is generally always preferred to have many
small texts instead of fewer but larger ones. This reduces the risk
for fuzzy strings (you change a minor part in a message and the
whole text is affected, needs to be retranslated) and could easily
allow reusing texts in different contexts.
> > Also formating strings should be factored out, e.g. via a format string
> > such as "%s (%c) %s" where the first %s is some kind of space, %c could
> > be a shortcut and %s the menu text. So one would need to translate this
> > format specification only once and with a minor single change all lines
> > (and maybe even different menus) can be adjusted at once.
>
> I'm OK with this. But it works only as long as we don't need
> the exact same format string for something else which is
> internationalised as well, right?
This would be no problem. If someone noticed that two identical strings
(e.g. "count" and "count") require different translations depending on
content one used a special function which ignored some parts of text.
For example:
P_("as used in phrase1|count")
P_("as used in phrase2|count")
const char *P_(const char *Text)
{
const char * const translation = gettext(Text);
const char * const stripto = strchr(translation, '|');
if(stripto == NULL)
return translation;
else
return stripto+1;
}
Translations: "as used in phrase1|count" ==> "Anzahl"
"as used in phrase2|count" ==> "Zahl"
Newer gettext version provide a dpgettext function that takes a context
argument.
> > Using some kind of format string could help. Some
> > libraries such as Qt use layout classes such as QVBox and QHBox
> > and align all childrens of these vertically or horizontally,
> > respectively. Complex layout can now be created by nesting these:
> > QVBox(QHBox(%s), QHBox(QVBox(%s))).
>
> I know you're not suggesting to, but I guess pulling in QT as
> a dependency to solve this issue is out of question :)
But an *own* format handling which fulfills svn needs would do it. I'm not
proposing this but the help message could be described by the following syntax:
(%B starts a block mode, %b ends it. Text in block mode will be breaked after
a fixed column at whitespaces and the indentation is kept.)
%command_name: %s.
usage: %s
%B%s%b
Valid options:
%s %move_to_column(15):%B%s%b
Jens
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] proper indentation in merge conflict menu
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Jun 06, 2008 at 03:56:57PM +0200, Jens Seidel wrote:
> On Fri, Jun 06, 2008 at 03:06:42PM +0200, Stefan Sperling wrote:
> > Note that probably not all systems we support have wide chars, so on
>
> I think one such system is cygwin.
>
> > What about maintaining multiple full text menus? The code could use
>
> I do not like text duplication. I have to translate it consistently.
We're currently talking of about 5 or 6 full text prompts that will share
most of their text. Each being about 3 lines long. I think this is a
negligible amount of additional translation overhead, considering that
it buys us consistent indentation for all languages on any platform
we support. And it keeps the code's complexity at the same level as it
currently is.
> Also formating strings should be factored out, e.g. via a format string
> such as "%s (%c) %s" where the first %s is some kind of space, %c could
> be a shortcut and %s the menu text. So one would need to translate this
> format specification only once and with a minor single change all lines
> (and maybe even different menus) can be adjusted at once.
I'm OK with this. But it works only as long as we don't need
the exact same format string for something else which is
internationalised as well, right?
> > This is very unfortunate. Doesn't gettext provide some way to deal
> > with these kinds of problems? How do other projects handle this?
>
> No, it doesn't.
:(
> Using some kind of format string could help. Some
> libraries such as Qt use layout classes such as QVBox and QHBox
> and align all childrens of these vertically or horizontally,
> respectively. Complex layout can now be created by nesting these:
> QVBox(QHBox(%s), QHBox(QVBox(%s))).
I know you're not suggesting to, but I guess pulling in QT as
a dependency to solve this issue is out of question :)
Stefan
Re: [PATCH] proper indentation in merge conflict menu
Posted by Jens Seidel <je...@users.sourceforge.net>.
On Fri, Jun 06, 2008 at 03:06:42PM +0200, Stefan Sperling wrote:
> Note that probably not all systems we support have wide chars, so on
I think one such system is cygwin.
> What about maintaining multiple full text menus? The code could use
I do not like text duplication. I have to translate it consistently.
Also formating strings should be factored out, e.g. via a format string
such as "%s (%c) %s" where the first %s is some kind of space, %c could
be a shortcut and %s the menu text. So one would need to translate this
format specification only once and with a minor single change all lines
(and maybe even different menus) can be adjusted at once.
> > > Do you know of any other places in Subversion that have this problem?
> >
> > There are really many strings which need to be tested live (not just by
> > proofreading the PO file).
This mainly affects the help texts.
> This is very unfortunate. Doesn't gettext provide some way to deal
> with these kinds of problems? How do other projects handle this?
No, it doesn't. Using some kind of format string could help. Some
libraries such as Qt use layout classes such as QVBox and QHBox
and align all childrens of these vertically or horizontally,
respectively. Complex layout can now be created by nesting these:
QVBox(QHBox(%s), QHBox(QVBox(%s))).
Jens
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] proper indentation in merge conflict menu
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Jun 06, 2008 at 09:49:28AM +0200, Jens Seidel wrote:
> On Thu, Jun 05, 2008 at 06:17:17PM -0400, Karl Fogel wrote:
> > Instead, perhaps we have to write multiline prompts in a way that allows
> > translations to work regardless of word length. I don't think counting
> > characters is the way to go, unless there's no other solution.
>
> Don't understand what's wrong with counting ... But it may indeed result
> in clumsy code.
The reason that it makes sense to do it in the cases you found is that
the code prints different parts of the menu depending on certain flags.
So the code assembles the whole menu from different pieces. The pieces
get translated individually and therefore each have different and unknown
length.
And you are right about clumsy code! "Counting characters" is very fragile.
BTW, if you look closely at my patch, you can see that "counting characters"
is actually not the proper term -- in some languages single characters
can take up more than one terminal column when printed. So instead of
"counting characters" we should probably say "determining string width
at run time."
Note that probably not all systems we support have wide chars, so on
these the indentation will still be wrong with the "run time" method.
I think it would be better to find some way to specify the amount of
indentation required in the data set (i.e. the strings) instead, and
to have the code be as stupid as possible ("just print it").
What about maintaining multiple full text menus? The code could use
a full text menu for each case it encounters, instead of building
up a suitable menu from several pieces. This would create some redundancy,
but would make the indentation work in all cases. After all, if
translators translate a full menu for each case, they can get the
indentation correct in all cases, right?
> > Do you know of any other places in Subversion that have this problem?
>
> There are really many strings which need to be tested live (not just by
> proofreading the PO file).
This is very unfortunate. Doesn't gettext provide some way to deal
with these kinds of problems? How do other projects handle this?
Stefan
Re: [PATCH] proper indentation in merge conflict menu
Posted by Jens Seidel <je...@users.sourceforge.net>.
On Thu, Jun 05, 2008 at 06:17:17PM -0400, Karl Fogel wrote:
> Whoa, whoa, whoa. Let's slow down and think about this carefully.
>
> I'm not sure r31600 is a generally applicable solution. After all, a
It's nevertheless fine for 1.5.0 as I only touched a PO file. Also this
message changed in trunk so we can slowly find a independent fix in trunk.
> translation might be longer instead of shorter.
You mean shorter instead of longer. In this case we could add additional
spaces to the translation of "Select: ".
> Instead, perhaps we have to write multiline prompts in a way that allows
> translations to work regardless of word length. I don't think counting
> characters is the way to go, unless there's no other solution.
Don't understand what's wrong with counting ... But it may indeed result
in clumsy code.
> Do you know of any other places in Subversion that have this problem?
There are really many strings which need to be tested live (not just by
proofreading the PO file). Consider e.g. help messages which often
have an indentation of 31 characters. Even if these 31 spaces are handled
separately the remaining text on this line should no longer than 80-31.
If a string is longer it needs to be wrapped and has to respect the
indendation.
Compare e.g.
$ LANG=fr_FR.utf8 svn help diff
-r [--revision] ARG : ARG (peut aussi être une étendue ARG1:ARG2)
L'argument d'une révision peut être :
NUMÉRO numéro de la révision
'{' DATE '}' révision disponible à cette date
--depth ARG : limite l'opétioan à cette profondeur (depth empty/files/
immediates/infinity) en argument
Such help messages use strings such as
#: ../svn/main.c:214
msgid "use ARG as diff command"
msgstr "utilise ARG comme commande diff"
and it is not easy to recognise that a longer translation has to be:
msgstr "long translation of \"use ARG as diff\n"
" command\" continued on a second line"
Jens
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] proper indentation in merge conflict menu
Posted by Daniel Shahaf <d....@daniel.shahaf.co.il>.
David Glasser wrote on Thu, 5 Jun 2008 at 20:06 -0700:
> We could just indent a fixed number of spaces, like one or two.
>
And add a linebreak between "Select: " and the first option:
- Select: (p) postpone
- (i) indented second line
+ Select:
+ (p) postpone
+ (i) indented second line
> Or we could translate the entire multi-line message, though that would
> require making the code flow differently.
>
> --dave
>
> On Thu, Jun 5, 2008 at 6:31 PM, Stefan Sperling <st...@elego.de> wrote:
> > On Thu, Jun 05, 2008 at 06:17:17PM -0400, Karl Fogel wrote:
> >> Whoa, whoa, whoa. Let's slow down and think about this carefully.
> >>
> >> I'm not sure r31600 is a generally applicable solution. After all, a
> >> translation might be longer instead of shorter.
> >>
> >> Instead, perhaps we have to write multiline prompts in a way that allows
> >> translations to work regardless of word length. I don't think counting
> >> characters is the way to go, unless there's no other solution.
> >
> > Regardless of whether or not we're going to count characters,
> > here's a diff that does the counting properly, just to illustrate
> > how this could be done.
> >
> > I *really* hope someone has a better idea. The amount of hoops
> > this patch has to jump through is a bit crazy. Maybe there's also
> > a way of doing this without wide chars, but I don't know how.
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] proper indentation in merge conflict menu
Posted by Stefan Sperling <st...@elego.de>.
On Fri, Jun 06, 2008 at 09:37:57AM -0700, David Glasser wrote:
> On Fri, Jun 6, 2008 at 6:13 AM, Stefan Sperling <st...@elego.de> wrote:
> > On Thu, Jun 05, 2008 at 08:06:07PM -0700, David Glasser wrote:
> >> We could just indent a fixed number of spaces, like one or two.
> >
> > We're currently already indenting a fixed number of spaces,
> > namely 8. What difference would tweaking that number make?
>
> Well, the problem here is that the indent is supposed to look like
> "Select: ", right? So if we stop caring about that...
Ah, right. I get your point now.
Stefan
Re: [PATCH] proper indentation in merge conflict menu
Posted by David Glasser <gl...@davidglasser.net>.
On Fri, Jun 6, 2008 at 6:13 AM, Stefan Sperling <st...@elego.de> wrote:
> On Thu, Jun 05, 2008 at 08:06:07PM -0700, David Glasser wrote:
>> We could just indent a fixed number of spaces, like one or two.
>
> We're currently already indenting a fixed number of spaces,
> namely 8. What difference would tweaking that number make?
Well, the problem here is that the indent is supposed to look like
"Select: ", right? So if we stop caring about that...
--dave
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] proper indentation in merge conflict menu
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Jun 05, 2008 at 08:06:07PM -0700, David Glasser wrote:
> We could just indent a fixed number of spaces, like one or two.
We're currently already indenting a fixed number of spaces,
namely 8. What difference would tweaking that number make?
> Or we could translate the entire multi-line message, though that would
> require making the code flow differently.
What you mean by this is, I think, the same thing I just described in
a more elaborate fashion in a another mail in this thread (maintaining
multiple versions of the menu, one for each case the code needs).
So, +1 to the latter proposal.
Stefan
Re: [PATCH] proper indentation in merge conflict menu
Posted by David Glasser <gl...@davidglasser.net>.
We could just indent a fixed number of spaces, like one or two.
Or we could translate the entire multi-line message, though that would
require making the code flow differently.
--dave
On Thu, Jun 5, 2008 at 6:31 PM, Stefan Sperling <st...@elego.de> wrote:
> On Thu, Jun 05, 2008 at 06:17:17PM -0400, Karl Fogel wrote:
>> Whoa, whoa, whoa. Let's slow down and think about this carefully.
>>
>> I'm not sure r31600 is a generally applicable solution. After all, a
>> translation might be longer instead of shorter.
>>
>> Instead, perhaps we have to write multiline prompts in a way that allows
>> translations to work regardless of word length. I don't think counting
>> characters is the way to go, unless there's no other solution.
>
> Regardless of whether or not we're going to count characters,
> here's a diff that does the counting properly, just to illustrate
> how this could be done.
>
> I *really* hope someone has a better idea. The amount of hoops
> this patch has to jump through is a bit crazy. Maybe there's also
> a way of doing this without wide chars, but I don't know how.
>
> [[[
>
> Fix indentation for translations of the interactive merge conflict
> resolution menu.
>
> * subversion/svn/conflict-callbacks.c
> (calc_prompt_indentation): New function.
> (svn_cl__conflict_handler): Try to indent the multi-line menu
> properly, regardless of the active language.
>
> * configure.ac: Check for wide char support.
>
> Found by: jensseidel
>
> ]]]
>
> Index: subversion/svn/conflict-callbacks.c
> ===================================================================
> --- subversion/svn/conflict-callbacks.c (revision 31609)
> +++ subversion/svn/conflict-callbacks.c (working copy)
> @@ -35,6 +35,10 @@
>
> #include "svn_private_config.h"
>
> +#ifdef HAVE_WCHAR_H
> +#include <wchar.h>
> +#endif
> +
>
>
>
> @@ -224,7 +228,37 @@ launch_resolver(svn_boolean_t *performed_edit,
> return SVN_NO_ERROR;
> }
>
> +/* Try to calculate the right amount of whitespace characters
> + * needed to indent up to the length of the string PROMPT.
> + * The right amount depends on the current language.
> + */
> +static int
> +calc_prompt_indentation(const char *prompt, apr_pool_t *pool)
> +{
> + int len = strlen(prompt); /* Default to byte count. */
>
> +#if defined(HAVE_MBSTOWCS) && defined(HAVE_WCSWIDTH)
> + /* We have wide chars, so try to be safe for multi-byte characters. */
> + int num_wchars = mbstowcs(NULL, prompt, 0);
> + if (num_wchars > 0)
> + {
> + /* From http://www.cl.cam.ac.uk/~mgk25/unicode.html:
> + * With UTF-8, neither a byte nor a character count will predict
> + * the display width, because ideographic characters (Chinese,
> + * Japanese, Korean) will occupy two column positions, whereas
> + * control and combining characters occupy none.
> + * To determine the width of a string on the terminal screen,
> + * it is necessary to decode the UTF-8 sequence and then use
> + * [...] wcswidth to measure the entire string. */
> + wchar_t *wprompt = apr_palloc(pool, sizeof(wchar_t) * (num_wchars + 1));
> + if (mbstowcs(wprompt, prompt, num_wchars) > 0)
> + len = wcswidth(wprompt, num_wchars);
> + }
> +#endif
> +
> + return len;
> +}
> +
> /* Implement svn_wc_conflict_resolver_func_t; resolves based on
> --accept option if given, else by prompting. */
> svn_error_t *
> @@ -364,6 +398,10 @@ svn_cl__conflict_handler(svn_wc_conflict_result_t
> {
> const char *answer;
> char *prompt;
> + char *select = _("Select: ");
> + svn_stringbuf_t *whitespace;
> + apr_size_t indent_len;
> + int i;
> svn_boolean_t diff_allowed = FALSE;
> /* Have they done something that might have affected the merged
> file (so that we need to save a .edited copy)? */
> @@ -423,11 +461,18 @@ svn_cl__conflict_handler(svn_wc_conflict_result_t
> || (!desc->base_file && desc->my_file && desc->their_file))
> diff_allowed = TRUE;
>
> + /* Create suitable amount of whitespace for prompt indentation. */
> + indent_len = calc_prompt_indentation(select, subpool);
> + whitespace = svn_stringbuf_create_ensure(indent_len + 1, pool);
> + for (i = 0; i < indent_len; i++)
> + svn_stringbuf_appendcstr(whitespace, " ");
> +
> while (TRUE)
> {
> svn_pool_clear(subpool);
>
> - prompt = apr_pstrdup(subpool, _("Select: (p) postpone"));
> + prompt = apr_pstrdup(subpool, select);
> + prompt = apr_pstrcat(subpool, prompt, _("(p) postpone"), NULL);
>
> if (diff_allowed)
> {
> @@ -442,7 +487,8 @@ svn_cl__conflict_handler(svn_wc_conflict_result_t
> if (! desc->is_binary &&
> desc->kind != svn_wc_conflict_kind_property)
> prompt = apr_pstrcat(subpool, prompt,
> - _(",\n (mc) mine-conflict, "
> + ",\n", whitespace->data,
> + _("(mc) mine-conflict, "
> "(tc) theirs-conflict"),
> NULL);
> }
> @@ -451,13 +497,12 @@ svn_cl__conflict_handler(svn_wc_conflict_result_t
> if (knows_something)
> prompt = apr_pstrcat(subpool, prompt, _(", (r) resolved"),
> NULL);
> - prompt = apr_pstrcat(subpool, prompt,
> - _(",\n "
> - "(mf) mine-full, (tf) theirs-full"),
> + prompt = apr_pstrcat(subpool, prompt, ",\n", whitespace->data,
> + _("(mf) mine-full, (tf) theirs-full"),
> NULL);
> }
>
> - prompt = apr_pstrcat(subpool, prompt, ",\n ", NULL);
> + prompt = apr_pstrcat(subpool, prompt, ",\n", whitespace->data, NULL);
> prompt = apr_pstrcat(subpool, prompt,
> _("(s) show all options: "),
> NULL);
> Index: configure.ac
> ===================================================================
> --- configure.ac (revision 31609)
> +++ configure.ac (working copy)
> @@ -610,7 +610,10 @@ AC_FUNC_VPRINTF
> dnl check for functions needed in special file handling
> AC_CHECK_FUNCS(symlink readlink)
>
> -
> +dnl Check for wide character support functions
> +AC_CHECK_HEADERS(wchar.h)
> +AC_CHECK_FUNCS(mbstowcs wcswidth)
> +
> dnl Process some configuration options ----------
>
> AC_ARG_WITH(ssl,
>
>
> So watcha think?
>
> Stefan
>
--
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] proper indentation in merge conflict menu
Posted by Stefan Sperling <st...@elego.de>.
On Thu, Jun 05, 2008 at 06:17:17PM -0400, Karl Fogel wrote:
> Whoa, whoa, whoa. Let's slow down and think about this carefully.
>
> I'm not sure r31600 is a generally applicable solution. After all, a
> translation might be longer instead of shorter.
>
> Instead, perhaps we have to write multiline prompts in a way that allows
> translations to work regardless of word length. I don't think counting
> characters is the way to go, unless there's no other solution.
Regardless of whether or not we're going to count characters,
here's a diff that does the counting properly, just to illustrate
how this could be done.
I *really* hope someone has a better idea. The amount of hoops
this patch has to jump through is a bit crazy. Maybe there's also
a way of doing this without wide chars, but I don't know how.
[[[
Fix indentation for translations of the interactive merge conflict
resolution menu.
* subversion/svn/conflict-callbacks.c
(calc_prompt_indentation): New function.
(svn_cl__conflict_handler): Try to indent the multi-line menu
properly, regardless of the active language.
* configure.ac: Check for wide char support.
Found by: jensseidel
]]]
Index: subversion/svn/conflict-callbacks.c
===================================================================
--- subversion/svn/conflict-callbacks.c (revision 31609)
+++ subversion/svn/conflict-callbacks.c (working copy)
@@ -35,6 +35,10 @@
#include "svn_private_config.h"
+#ifdef HAVE_WCHAR_H
+#include <wchar.h>
+#endif
+
@@ -224,7 +228,37 @@ launch_resolver(svn_boolean_t *performed_edit,
return SVN_NO_ERROR;
}
+/* Try to calculate the right amount of whitespace characters
+ * needed to indent up to the length of the string PROMPT.
+ * The right amount depends on the current language.
+ */
+static int
+calc_prompt_indentation(const char *prompt, apr_pool_t *pool)
+{
+ int len = strlen(prompt); /* Default to byte count. */
+#if defined(HAVE_MBSTOWCS) && defined(HAVE_WCSWIDTH)
+ /* We have wide chars, so try to be safe for multi-byte characters. */
+ int num_wchars = mbstowcs(NULL, prompt, 0);
+ if (num_wchars > 0)
+ {
+ /* From http://www.cl.cam.ac.uk/~mgk25/unicode.html:
+ * With UTF-8, neither a byte nor a character count will predict
+ * the display width, because ideographic characters (Chinese,
+ * Japanese, Korean) will occupy two column positions, whereas
+ * control and combining characters occupy none.
+ * To determine the width of a string on the terminal screen,
+ * it is necessary to decode the UTF-8 sequence and then use
+ * [...] wcswidth to measure the entire string. */
+ wchar_t *wprompt = apr_palloc(pool, sizeof(wchar_t) * (num_wchars + 1));
+ if (mbstowcs(wprompt, prompt, num_wchars) > 0)
+ len = wcswidth(wprompt, num_wchars);
+ }
+#endif
+
+ return len;
+}
+
/* Implement svn_wc_conflict_resolver_func_t; resolves based on
--accept option if given, else by prompting. */
svn_error_t *
@@ -364,6 +398,10 @@ svn_cl__conflict_handler(svn_wc_conflict_result_t
{
const char *answer;
char *prompt;
+ char *select = _("Select: ");
+ svn_stringbuf_t *whitespace;
+ apr_size_t indent_len;
+ int i;
svn_boolean_t diff_allowed = FALSE;
/* Have they done something that might have affected the merged
file (so that we need to save a .edited copy)? */
@@ -423,11 +461,18 @@ svn_cl__conflict_handler(svn_wc_conflict_result_t
|| (!desc->base_file && desc->my_file && desc->their_file))
diff_allowed = TRUE;
+ /* Create suitable amount of whitespace for prompt indentation. */
+ indent_len = calc_prompt_indentation(select, subpool);
+ whitespace = svn_stringbuf_create_ensure(indent_len + 1, pool);
+ for (i = 0; i < indent_len; i++)
+ svn_stringbuf_appendcstr(whitespace, " ");
+
while (TRUE)
{
svn_pool_clear(subpool);
- prompt = apr_pstrdup(subpool, _("Select: (p) postpone"));
+ prompt = apr_pstrdup(subpool, select);
+ prompt = apr_pstrcat(subpool, prompt, _("(p) postpone"), NULL);
if (diff_allowed)
{
@@ -442,7 +487,8 @@ svn_cl__conflict_handler(svn_wc_conflict_result_t
if (! desc->is_binary &&
desc->kind != svn_wc_conflict_kind_property)
prompt = apr_pstrcat(subpool, prompt,
- _(",\n (mc) mine-conflict, "
+ ",\n", whitespace->data,
+ _("(mc) mine-conflict, "
"(tc) theirs-conflict"),
NULL);
}
@@ -451,13 +497,12 @@ svn_cl__conflict_handler(svn_wc_conflict_result_t
if (knows_something)
prompt = apr_pstrcat(subpool, prompt, _(", (r) resolved"),
NULL);
- prompt = apr_pstrcat(subpool, prompt,
- _(",\n "
- "(mf) mine-full, (tf) theirs-full"),
+ prompt = apr_pstrcat(subpool, prompt, ",\n", whitespace->data,
+ _("(mf) mine-full, (tf) theirs-full"),
NULL);
}
- prompt = apr_pstrcat(subpool, prompt, ",\n ", NULL);
+ prompt = apr_pstrcat(subpool, prompt, ",\n", whitespace->data, NULL);
prompt = apr_pstrcat(subpool, prompt,
_("(s) show all options: "),
NULL);
Index: configure.ac
===================================================================
--- configure.ac (revision 31609)
+++ configure.ac (working copy)
@@ -610,7 +610,10 @@ AC_FUNC_VPRINTF
dnl check for functions needed in special file handling
AC_CHECK_FUNCS(symlink readlink)
-
+dnl Check for wide character support functions
+AC_CHECK_HEADERS(wchar.h)
+AC_CHECK_FUNCS(mbstowcs wcswidth)
+
dnl Process some configuration options ----------
AC_ARG_WITH(ssl,
So watcha think?
Stefan