From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS53758 23.128.96.0/24 X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id 652841F9F4 for ; Mon, 13 Sep 2021 00:13:50 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236849AbhIMAPB (ORCPT ); Sun, 12 Sep 2021 20:15:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50486 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236824AbhIMAOz (ORCPT ); Sun, 12 Sep 2021 20:14:55 -0400 Received: from mail-wm1-x32a.google.com (mail-wm1-x32a.google.com [IPv6:2a00:1450:4864:20::32a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 48E89C06175F for ; Sun, 12 Sep 2021 17:13:40 -0700 (PDT) Received: by mail-wm1-x32a.google.com with SMTP id e26so5381325wmk.2 for ; Sun, 12 Sep 2021 17:13:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=p/ksR24wY5NweedgSLwFEFicXt35ubVv9roIxSZe6U0=; b=qJ38w7C/lpTSjfHGhklBSVqkoqwtAIJr+iiFYYjl08DSYloO3yF2iDI+J5dmeqzVs3 LSF6SsWjXtdzhCsrKQlxZmenX54KeB177TW9kgdQWn2CBmYty2u0+yBcCg+R6O//GUec ggF+E+TOs5fHUnU6uebK0UU+S4DFBpRBdWxT7M6ggW1lIJCgX8ZsFoABuB9hqrlJ0pAh zkrQEvnodm77ufj1u3xQPQ++W59vF5mC6GSaCWqZiwSTk4hfVC+upfwnT+3U/cRPWQ2Z l+CLPxcrGGv0qCfrvMZf6qiwenuJVfvL1N+gIz13GFtbTb39A52/6KMYGikcgL2Hbc5u m4DQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=p/ksR24wY5NweedgSLwFEFicXt35ubVv9roIxSZe6U0=; b=dmKMg1/w3jgLdmP+8FDdMZQnsFE0rITUSBT0fdtpzKCWaRyagngHSbuSrG8qIK3DaE nup6qB7j/auEBqYMNW4Owmr7nTL6RkRl+nuryhb9u0+3P13GcWb9NLXofJDzJ8GttNAC hQN/ampx5dr+Y7IWWZAx1b6v/VJdm8TsNFKe9FAhpJMaUAkXn6SYO654Xx6/3UxR9Vrl C4CUDVILCFlxz9FlHJXMK49JXsutiSgkYDjgrG1zp0YiTaPmP5rx0n51c19RJoA+CIcR 7QA4Vzysvtl2DDRTN4K7g7ITOoY5YhyjRwKlGSxjnVjUpH2hHGP/GnCbb9GYObtzK6Qv iJkA== X-Gm-Message-State: AOAM532ZROrMPx8Eoh9s4lwtSj/UpmMmQnVHGbqlhzlY/xoLbe5nm6qD 9maqp/xNM2hP1q0ArFqXMIT9jNozRpSE9w== X-Google-Smtp-Source: ABdhPJyNMzyaFoR5TjPQ6WZC22+5rzpxLx3J9wlObS3dSkj3+gkywLyMJBvQqqjmMIrfkbgfSyJvSA== X-Received: by 2002:a1c:3584:: with SMTP id c126mr8551905wma.121.1631492018582; Sun, 12 Sep 2021 17:13:38 -0700 (PDT) Received: from vm.nix.is (vm.nix.is. [2a01:4f8:120:2468::2]) by smtp.gmail.com with ESMTPSA id j14sm5827974wrp.21.2021.09.12.17.13.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 12 Sep 2021 17:13:38 -0700 (PDT) From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= To: git@vger.kernel.org Cc: Junio C Hamano , Jeff King , Carlo Arenas , Eric Sunshine , =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Subject: [PATCH v4 4/4] parse-options: properly align continued usage output Date: Mon, 13 Sep 2021 02:13:22 +0200 Message-Id: X-Mailer: git-send-email 2.33.0.1001.g3ab2ac1eaae In-Reply-To: References: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Some commands such as "git stash" emit continued options output with e.g. "git stash -h", because usage_with_options_internal() prefixes with its own whitespace the resulting output wasn't properly aligned. Let's account for the added whitespace, which properly aligns the output. The "git stash" command has usage output with a N_() translation that legitimately stretches across multiple lines; N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n" " [-u|--include-untracked] [-a|--all] [-m|--message ]\n" [...] We'd like to have that output aligned with the length of the initial "git stash " output, but since usage_with_options_internal() adds its own whitespace prefixing we fell short, before this change we'd emit: $ git stash -h usage: git stash list [] or: git stash show [] [] [...] or: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet] [-u|--include-untracked] [-a|--all] [-m|--message ] [...] Now we'll properly emit aligned output. I.e. the last four lines above will instead be (a whitespace-only change to the above): [...] or: git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet] [-u|--include-untracked] [-a|--all] [-m|--message ] [...] We could also go for an approach where we have the caller support no padding of their own, i.e. (same as the first example, except for the padding on the second line): N_("git stash [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]\n" "[-u|--include-untracked] [-a|--all] [-m|--message ]\n" [...] But to do that we'll need to find the length of "git stash". We can discover that from the "cmd" in the "struct cmd_struct", but there might cases with sub-commands or "git" itself taking arguments that would make that non-trivial. Even if it was I still think this approach is better, because this way we'll get the same legible alignment in the C code. The fact that usage_with_options_internal() is adding its own prefix padding is an implementation detail that callers shouldn't need to worry about. Implementation notes: We could skip the string_list_split() with a strchr(str, '\n') check, but we'd then need to duplicate our state machine for strings that do and don't contain a "\n". It's simpler to just always split into a "struct string_list", even though the common case is that that "struct string_list" will contain only one element. This is not performance-sensitive code. This change is relatively more complex since I've accounted for making it future-proof for RTL translation support. Later in usage_with_options_internal() we have some existing padding code dating back to d7a38c54a6c (parse-options: be able to generate usages automatically, 2007-10-15) which isn't RTL-safe, but that code would be easy to fix. Let's not introduce new RTL translation problems here. Signed-off-by: Ævar Arnfjörð Bjarmason --- parse-options.c | 76 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 64 insertions(+), 12 deletions(-) diff --git a/parse-options.c b/parse-options.c index 2abff136a17..75f0a6c81c5 100644 --- a/parse-options.c +++ b/parse-options.c @@ -917,25 +917,77 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx, FILE *outfile = err ? stderr : stdout; int need_newline; + const char *usage_prefix = _("usage: %s"); + /* + * The translation could be anything, but we can count on + * msgfmt(1)'s --check option to have asserted that "%s" is in + * the translation. So compute the length of the "usage: " + * part. We are assuming that the translator wasn't overly + * clever and used e.g. "%1$s" instead of "%s", there's only + * one "%s" in "usage_prefix" above, so there's no reason to + * do so even with a RTL language. + */ + size_t usage_len = strlen(usage_prefix) - strlen("%s"); + /* + * TRANSLATORS: the colon here should align with the + * one in "usage: %s" translation. + */ + const char *or_prefix = _(" or: %s"); + /* + * TRANSLATORS: You should only need to translate this format + * string if your language is a RTL language (e.g. Arabic, + * Hebrew etc.), not if it's a LTR language (e.g. German, + * Russian, Chinese etc.). + * + * When a translated usage string has an embedded "\n" it's + * because options have wrapped to the next line. The line + * after the "\n" will then be padded to align with the + * command name, such as N_("git cmd [opt]\n<8 + * spaces>[opt2]"), where the 8 spaces are the same length as + * "git cmd ". + * + * This format string prints out that already-translated + * line. The "%*s" is whitespace padding to account for the + * padding at the start of the line that we add in this + * function. The "%s" is a line in the (hopefully already + * translated) N_() usage string, which contained embedded + * newlines before we split it up. + */ + const char *usage_continued = _("%*s%s"); + const char *prefix = usage_prefix; + int saw_empty_line = 0; + if (!usagestr) return PARSE_OPT_HELP; if (!err && ctx && ctx->flags & PARSE_OPT_SHELL_EVAL) fprintf(outfile, "cat <<\\EOF\n"); - fprintf_ln(outfile, _("usage: %s"), _(*usagestr++)); - while (*usagestr && **usagestr) - /* - * TRANSLATORS: the colon here should align with the - * one in "usage: %s" translation. - */ - fprintf_ln(outfile, _(" or: %s"), _(*usagestr++)); while (*usagestr) { - if (**usagestr) - fprintf_ln(outfile, _(" %s"), _(*usagestr)); - else - fputc('\n', outfile); - usagestr++; + const char *str = _(*usagestr++); + struct string_list list = STRING_LIST_INIT_DUP; + unsigned int j; + + if (!saw_empty_line && !*str) + saw_empty_line = 1; + + string_list_split(&list, str, '\n', -1); + for (j = 0; j < list.nr; j++) { + const char *line = list.items[j].string; + + if (saw_empty_line && *line) + fprintf_ln(outfile, _(" %s"), line); + else if (saw_empty_line) + fputc('\n', outfile); + else if (!j) + fprintf_ln(outfile, prefix, line); + else + fprintf_ln(outfile, usage_continued, + (int)usage_len, "", line); + } + string_list_clear(&list, 0); + + prefix = or_prefix; } need_newline = 1; -- 2.33.0.1001.g3ab2ac1eaae