Re: [PATCH] proper indentation in merge conflict menu

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 <stsp_at_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_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org