Update and mark message strings for translation by chrisd8088 · Pull Request #4846 · git-lfs/git-lfs

chrisd8088 · 2022-01-30T08:18:50Z

Following from the work in PR #4781 and #4729, we mark a number of additional message strings for translation, and revise various message strings with the aim of improving their consistency and clarity.

This PR may be most easily reviewed in commit-by-commit fashion, although it should also be readable as a single diff.

Aside from adding messages to the set of translatable string, which we undertake in the final commit, the preceding commits attempt to address a number of issues, including:

Using consistent capitalization for acronyms, abbreviations, and project names (e.g., "Git" and "Git LFS").
- Note that this PR does not attempt to ensure all messages begin with a capitalized (or all-lowercase) first word.
Improving our grammar and punctuation in a few messages.
Removing short message prefixes where they are used inconsistently within a package and do not add particular value.
- We leave these types of prefixes in place in trace messages intended for debugging, however, as they have utility there.
Moving other short message prefixes specific to a Git LFS command, such as migrate:, out of the translatable strings, as they are intended to reference they command's name.
Using backticks to delimit git ... and git lfs ... commands where they appear in messages.
Using single quotes to delimit file names and paths, e.g., '.gitattributes', where they appear in messages.
- This PR does not achieve perfect consistency in this regard, though, as many message string formats use %q to quote user-supplied file paths, Git remotes, Git ref names, etc.
Making some strings untranslatable, either because they appear in Go test files, or because they are fixed strings such as Git LFS command examples, Git configuration examples, etc.
Moving most newlines outside of the translatable strings, as this should aid the task of ensuring translations have equivalent formatting, and because we can often make use of the formatting capabilities of the function enclosing the call to tr.Tr.Get() to do so.
Dropping or changing the functions enclosing calls to tr.Tr.Get() to ensure we never pass a translated and interpolated string as another format string.

Re the last item, because the tr.Tr.Get() family of methods insert arguments into printf(3)-style format strings after translating the format string, we can in a few cases drop a surrounding call to fmt.Sprintf() or a similar method, as those now take no arguments and so are redundant. This will help us avoid situations where either the translated string or the argument values interpolated by tr.Tr.Get() produce an output string which itself happens to contain character sequences that resemble Go format specifiers (e.g., %s, %d, etc.) In such cases passing the string at runtime to a method such as fmt.Fprintf() will result in the output containing a warning such as %!s(MISSING), which is not ideal.

As an example, if pointerFile contained %d then the output from fmt.Fprintf() would contain %!d(MISSING):

	fmt.Fprintf(os.Stderr, tr.Tr.Get("Git LFS pointer for %s\n\n", pointerFile))

In one case, in lfs/attribute.go, we can now also simplify the format string to use standard format specifiers instead of double-escaped ones (e.g., %%q) since we can just allow tr.Tr.Get() to do the interpolation.

We also take the opportunity to remove explicit leading or trailing newlines from translation messages wherever it is possible to convert the enclosing function to fmt.Print(), fmt.Println(), etc., noting that the former can be used to concatenate multiple strings without intermediate spaces, while the latter will insert spaces between all its arguments.

Within the commands package we can also make use of the formatting provided by the Print(), Error(), etc. functions, as they ultimately utilize fmt.Fprintln() when no arguments are provided, so a newline is automatically appended, and when arguments are provided, the fmt.Fprintf() is used but these utility functions append a newline for us to the format string we pass. But in that instance we then need to be careful not to pass our translated string as the format string, but as an argument, as noted above.

Lastly, we include panic messages amongst our translated strings as there is already precedent for translating these.

We separate the command-specific prefix output by the "git lfs migrate" command from language-specific text strings in several messages. This is in keeping with the formatting used for all other instances of this message prefix.

Several error messages output by the "git lfs lock" command currently begin with an "lfs:" prefix, which is unlike any other error messages from that command (or from other commands, for that matter). We can therefore simplify our message strings slightly by simply removing this prefix from them. Note that two of these messages are not yet passed as translation strings, but we will address that issue in a subsequent commit.

Several error messages output by the "git lfs track" command and the transfer queue currently begin with a "Git LFS:" prefix, which is unlike any other error messages from either that command or that package, or anywhere else, for that matter. We can therefore simplify our translation message strings slightly by simply removing this prefix from them.

Several error messages output by the git package currently begin with a "git:" prefix, which is unlike any other error messages from that package, or anywhere else, for that matter. We can therefore simplify our message strings slightly by simply removing this prefix from them, and in one case, we reword the error message to provide the full text of the failing Git command (git for-each-ref), which is how other Git errors are reported. Note that these messages are not yet passed as translation strings, but we will address that issue in a subsequent commit.

One error message output by the git/gitattr package currently begins with a "git/gitattr:" prefix, which is unlike other error messages from that package, except for one where the entire message consists of just the package name, and which we will replace in a subsequent commit. We can therefore simplify the message string with the prefix simply removing the prefix. Note that this message is not yet passed as a translation string, but we will address that issue in a subsequent commit.

Each of the upload transfer queue adapters reports an error with a similar message in the case of a 403 response, and all of these messages begin with an "http:" prefix (even the one in the SSH adapter). However, other status codes are not reported the same way. This prefix should either be removed from the translatable message text, or removed entirely; we choose the latter option here as the simplest. We also now interpolate the status code rather than make it a fixed part of the translatable string, and start the messages with a capital letter.

One error message output by the transfer queue currently begins with a "tq:" prefix, which is unlike any other error messages from that package, or from anywhere else, for that matter. We can therefore simplify our message string slightly by simply removing this prefix from it. Note that this message is not yet passed as a translation string, but we will address that issue in a subsequent commit.

Akin to several other commands such as "git lfs migrate" and "git lfs prune", the "git lfs fetch" command prefixes a number of its information messages with the name of the command, i.e., with the "fetch:" prefix. To align with those other commands, we therefore remove this command-specific prefix from our translatable message strings and make it a fixed prefix for all languages.

We rephrase some error message strings which are used when wrapping other errors so they are clearer and more consistent with other messages. Note that these strings will be prepended to the wrapper errors' messages. In the lfs/diff_index_scanner.go file in particular we rephrase the additional message to include a full Git command ("git diff-index"), which is similar to how errors are reported in the git package.

Several panic() calls in the basic and tus.io transfer queue adapaters have very similar messages, which we can make more consistent with each other; this will ease translation support when we make these messages translatable in a subsequent commit.

We rephrase and repunctuate a few error and informational message strings so they are clearer and more consistent with our other messages. In particular, in the git/gitattr package we replace one message which consisted solely of the package's name with a more explanatory error message.

We revise two error message format strings so as to use the %q format specifier for Git ref names, which is how another error message in the commands/lockverifier.go file quotes a ref name. This is also more consistent with how the majority of our error messages format Git remote names. While not entirely ideal, this change at least ensures we handle ref names consistently.

We convert several log messages to use the uppercase acronym "HTTP" instead of the lowercase variant. We also edit one message in lfshttp/client.go to remove an idiosyncratic prefix of the full source code file name, and edit the message slightly for greater clarity.

We convert a log message to use the uppercase acronym "JSON" instead of the lowercase variant.

We convert a log message to use the uppercase acronym "URL" instead of the lowercase variant. Note that this message is not yet passed as a translation string, but we will address that issue in a subsequent commit.

We convert several log messages to use the uppercase acronym "OID" instead of the lowercase variant, in one instance replacing the phrase "object id". Note that one of these messages is not yet passed as a translation string, but we will address that issue in a subsequent commit.

We convert a log message to use the uppercase abbreviation "ID" instead of the lowercase variant.

We convert several log messages to use the uppercase acronym "LFS" instead of the lowercase variant. Note that one of these messages is not yet passed as a translation string, but we will address that issue in a subsequent commit.

We convert several log messages to use the uppercase names "Windows" and "Cygwin" instead of all-lowercase variants.

We convert a few messages output by the various commands to use the full "Git LFS" project name instead of the shorter "LFS" variant, which will make them consistent with how the project name appears in other messages elsewhere. Note that we leave the status messages in the transfer queue using the shorter variant as they are intended to be brief, are checked extensively in the test suite, and are familiar to users at this point.

We convert a few messages output by the various commands to use the capitalized "Git" project name instead of the lowercase variant, which will make them consistent with how the Git project name appears in other messages elsewhere. Note that some of these messages are not yet passed as translation strings, but we will address that issue in a subsequent commit.

We convert one message output by the git package to use the lowercase "git" binary name instead of the capitalized variant, which will make this message consistent with how other Git commands are reported in error messages elsewhere. Note that this message is not yet passed as a translation string, but we will address that issue in a subsequent commit.

We convert several output messages to use a spelling of "de-duplication" consistent with all other messages from the "git lfs dedup" command. We also revise one message's text to clarify that it refers to a Git LFS object file. Note that one of these messages is not yet passed as a translation string, but we will address that issue in a subsequent commit.

In commit 1ddc2f2 of PR git-lfs#4781 text translation support was added for the "git lfs ext" command and a multi-line message was concatenated. However, the latter part of this text should be fixed for all users, since it displays a listing of the extension's settings values using defined terms. We therefore separate the first line of the message and let that be translated, as it contains the language-specific header text.

In one message string we can simplify the translation task by moving the fixed formatting (indicating pushed file OIDs and names) outside of the text string to be translated.

In commit ff0f378 of PR git-lfs#4781 a usage message for the "git lfs untrack" command was made translatable; however, since this only contains a fixed command example which references the man pages, we can avoid translating it.

In commit c744476 of PR git-lfs#4781 first part of a multi-line message was made translatable, while the second part, which consists only of an example "git config" command, was not. That change is reasonable; however, some indentation whitespace preceding the example command was lost, so we restore it here.

In commit 08774bf of PR git-lfs#4781 two message strings in the tq/api_test.go file were made translatable; however, since this only contains Go tests, we can reverse that change.

In the logPanicToWriter() method a "le" variable containing an OS-specific line ending string (e.g., "\n" on Unix/Linux) is prepended and appended to various strings using the string concatentation operator. However, all these instances occur within fmt.Fprint() and fmt.Fprintf() calls, so we can simply use those tools to perform the concatenations. These changes will reduce the changes needed in a subsequent commit when we make some of these strings translatable.

Because the tr.Tr.Get() family of methods insert arguments into printf(3)-style format strings after translating the format string, we can in a few cases drop a surrounding call to fmt.Sprintf() or a similar method, as those now take no arguments and so are redundant. Moreover, this will help us avoid situations where either the translated string or the argument values interpolated by tr.Tr.Get() produce an output string which itself happens to contain character sequences that resemble Go format specifiers (e.g., "%s", "%d", etc.) In such cases passing the string at runtime to a method such as fmt.Fprintf() will result in the output containing a warning such as "%!s(MISSING)", which is not ideal. Note that in one case, in lfs/attribute.go, we can now also simplify the format string to use standard format specifiers instead of double-escaped ones (e.g., "%%q") since we can just allow tr.Tr.Get() to do the interpolation. We also take the opportunity to remove explicit leading or trailing newlines from translation messages wherever it is possible to convert the surrounding call to fmt.Print(), fmt.Fprint(), fmt.Println(), or fmt.Fprintln(). Finally, in the commands/run.go file, we can replace two calls to fmt.Fprintf() with fmt.Println() because they are just printing output to os.Stdout, not os.Stderr, and in the lfs/extension.go file, we can make better use of fmt.Errorf(). Note that at least one of these messages is not yet actually passed as a translation string, but we will address that issue in a subsequent commit.

In general, we can make the translation task somewhat easier for translators, and less prone to error, if we remove non-language-specific newlines from the text strings to be translated. To do this we can in some cases simply drop newlines (e.g., in panic messages), or break multi-line text strings into several separate messages where that makes sense. For the most part we simply append the necessary trailing or intermediate newlines using the formatting tools available, especially those provided in the commands/commands.go file. Note that those tools (i.e., Print(), Exit(), etc.) always append a newline, but some messages are intended to have two final newlines, such as those from the "git lfs status" command. We also add a note to translators regarding some specific spacing required by a message output by the "git lfs dedup" command.

A number of message strings contain embedded Git or Git LFS commands, and while in many cases these are already delimited with backticks, in other cases they are not. We therefore rework the formatting of these messages to accord with the general practice of using backticks to delimit "git" and "git lfs" commands. In one case for the "git lfs clone" command this requires us to split a multi-line message into several parts, but that also has the advantage that we can move some of the fixed formatting and newlines out of the translatable message strings. Note that some of these messages are not yet passed as translation strings, but we will address that issue in a subsequent commit.

A number of message strings contain embedded filenames, and while in many cases these are already delimited with single quotes, in other cases they are not. We therefore rework the formatting of these messages to accord with the general practice of using single quotes to delimit specific filenames. Note that some of these messages are not yet passed as translation strings, but we will address that issue in a subsequent commit.

In the "git lfs smudge" and "git lfs status" commands we can slightly simplify the translation task for a pair of messages in which we expect to delimit the translated text in a fixed pair of angle brackets, so we move those outside the translation strings.

Following on from the changes in PR git-lfs#4781, we can make additional message strings translatable using the tr.Tr.Get() method. Because this method performs printf(3)-style format string parsing and interpolation, we can simplify some of the surrounding calls, e.g., from fmt.Errorf() to errors.New(), and from fmt.Fprintf() to fmt.Fprintln(). This ensures that if either the translated text or any interpolated arguments happen to contain character sequences that would be interpreted as Go format specifiers (e.g., "%s" or "%d"), these will not result in warnings such as "%!s(MISSING)" in the output text. Note also that we try to remove newlines from the message strings were possible and change the surrounding calls to append them instead, e.g., with fmt.Fprintln().

bk2204

This looks great! Thanks so much.

I had one minor thing I noticed, but I think this would be fine with or without that change.

commands/command_filter_process.go

Now that we have quantity-sensitive text messages for translation purposes, we can correct one message to use the singular form. h/t bk2204 on PR review.

We convert a message that is output when uninstalling Git hooks back to using a capitalized "Git" project name instead of the lowercase variant. This message was accidentally changed in commit 04abbd8 in PR git-lfs#4846 when making it into a translatable string.

In PR git-lfs#4781 we updated a number of packages to replace global string variables with local variables so they could be translated via the "tr" package's localization functions. One concern raised in that PR pertained to the changes to the lfshttp/errors.go source file, where a map of error format strings is constructed in the defaultError() function, with each string mapped to a specific HTTP status code. These strings should contain at least one "%s" verb, into which the HTTP request URL should be interpolated. In commit cda9c3c of PR git-lfs#4781 we moved the map's initialization into the defaultError() function of our "lfshttp" package, and added a call to the Get() method of the Tr structure from our "tr" package around each string's definition. However, we deferred the interpolation of the request URL to a subsequent call to a string formatting function, with the intent that we would first select the correct localized format string from the map based on the HTTP request's status code, and then interpolate the request's URL into the final error message. Further, if a request's status code does not correspond to a string in the map, our defaultError() function creates a generic error message into which we try to interpolate both the request's URL and its status code. For these generic messages, the intent of the changes in commit cda9c3c was that we first interpolate the status code, and then treat the resultant string as if it had been found in the defaultErrors map, meaning it should still contain one "%s" verb into which the request URL can be interpolated. Such a design implies that in the generic error messages we need to use a "%d" verb for the status code, and a doubly-escaped "%%s" verb for the request URL, since we want the latter to be converted to a regular "%s" verb when the string is first used as a format string. This design exposed an unexpected issue, which is described in leonelquinteros/gotext#57. In trying to work around this issue, though, we introduced a bug of our own whereby the generic messages are passed through three formatting functions rather than two, and in doing so the "%%s" verbs are eliminated before we attempt to interpolate the HTTP request's URL, leading to further errors. The single commit in PR git-lfs#5822 attempts to fix this problem, but without complete success. We now adopt the changes from that PR's commit and resolve the remaining problems so we should always supply an appropriate set of arguments for the number of formatting verbs in both types of messages in the defaultError() function. The confusion here stems from the issue reported in leonelquinteros/gotext#57, namely that the Printf() function of the "github.com/leonelquinteros/gotext" package behaves differently depending on whether or not it is passed any arguments other than a format string. When no arguments are passed, the format string is not parsed and any verbs are left unaltered, whereas when at least one additional argument is passed, the format string is parsed and the trailing arguments interpolated into it. The Tr global variable in our "tr" package is a Locale structure from the "gotext" package. Starting with commit cda9c3c, we separately invoke this variable's Get() method on each of the error messages defined in the defaultErrors map, but without any other arguments. Because the Get() method does not parse the format string when no other arguments are passed, in theory we ought to be able to define these messages with regular "%s" verbs. If we do so, though, the Go linter complains that it appears we are passing format strings to the Get() method without matching arguments for their "%s" verbs. To work around this linter limitation, we currently use "%%s" verbs in the error messages in the defaultErrors map, which are then passed through the Get() method unchanged. We then make a subsequent call to the Sprintf() function from the Go standard library's "fmt" package, passing the error message as the format string but without any other arguments. This call effectively just converts any "%%s" verbs to "%s" verbs. By contrast, the generic error messages in the defaultError() function are passed to the Tr variable's Get() method with another argument, the HTTP request's status code. Due to the presence of the extra argument, the Get() method parses the format string, replacing the "%d" verb with the status code and converting the "%%s" verb into a "%s" verb. (Note that originally, in PR git-lfs#4781, we used the Sprintf() function of the standard "fmt" package rather than the Get() method of our Tr variable. We corrected this oversight in commit 04abbd8 of PR git-lfs#4846, but while that change allows the strings to be localized, it has no effect on the handling of the formatting verbs.) As with the error messages from the defaultErrors map, the generic error messages are then also passed without any other arguments to the Sprintf() function from the standard "fmt" package. However, this call does not have the same effect in both cases. When the error message is specific to an HTTP status code and has been selected from the defaultErrors map, this call just converts the "%%s" verb into a "%s" one, as noted above. When the error message is a generic one, though, that conversion has already been performed by the Tr structure's Get() method. The consequence is that the Sprintf() function replaces the "%s" verb with "%!s(MISSING)", as described in the "fmt" package's documentation: https://pkg.go.dev/fmt#hdr-Format_errors In the final step of the defaultError() function we then pass the result from this Sprintf() call to the Errof() function of our "errors" package, and here we include the HTTP request's URL as a second argument. In the case where the error message has been selected from the defaultErrors map, this last formatting function replaces the "%s" verb with the URL. In the case where the error message is a generic one, though, the leading portion of the format error marker "%!s(MISSING)" cannot be parsed as a verb, and so the "%!" is replaced with a full reflection of all the internal elements of the URL structure into string form. Since these format error markers and reflection fields are not desirable in our HTTP request error messages, we revise the defaultError() function so it each type of error message is passed through only a single formatting function, specifically the Tr variable's Get() method. This allows us to use regular "%s" and "%d" formatting verbs exclusively in all the messages. Rather than call the Get() method on each string as it is defined in the defaultErrors map, we only call the method once a specific message is selected based on the HTTP request's status code, and we pass the request's URL to the Get() method as another argument. For the generic error messages, we simply pass both the status code and the URL as arguments to the Get() function. Because we always pass either one or two arguments to that function in addition to a format string, it always performs the appropriate interpolation for us, and we avoid the need to work around its behaviour when it is only passed a format string without other arguments, as well as avoiding the use of any further formatting functions like Sprintf(). Co-authored-by: Philip Peterson <pc.peterso@gmail.com>

chrisd8088 added 30 commits January 26, 2022 22:59

commands: separate migrate prefix from messages

2eb80e5

We separate the command-specific prefix output by the "git lfs migrate" command from language-specific text strings in several messages. This is in keeping with the formatting used for all other instances of this message prefix.

tq: make panic messages consistent

d5bef9f

Several panic() calls in the basic and tus.io transfer queue adapaters have very similar messages, which we can make more consistent with each other; this will ease translation support when we make these messages translatable in a subsequent commit.

lfshttp/lfshttp.go: uppercase JSON in message

9dc5afb

We convert a log message to use the uppercase acronym "JSON" instead of the lowercase variant.

git/git.go: uppercase URL in message

7b3b188

We convert a log message to use the uppercase acronym "URL" instead of the lowercase variant. Note that this message is not yet passed as a translation string, but we will address that issue in a subsequent commit.

locking/locks.go: uppercase ID in message

7e4eaa0

We convert a log message to use the uppercase abbreviation "ID" instead of the lowercase variant.

lfs/pointer.go: uppercase LFS in messages

bf15df6

We convert several log messages to use the uppercase acronym "LFS" instead of the lowercase variant. Note that one of these messages is not yet passed as a translation string, but we will address that issue in a subsequent commit.

tools/os_tools.go: uppercase names in messages

ab10f81

We convert several log messages to use the uppercase names "Windows" and "Cygwin" instead of all-lowercase variants.

commands/uploader.go: separate translation text

04c85a0

In one message string we can simplify the translation task by moving the fixed formatting (indicating pushed file OIDs and names) outside of the text string to be translated.

commands/command_untrack.go: print fixed usage msg

6acd3e4

In commit ff0f378 of PR git-lfs#4781 a usage message for the "git lfs untrack" command was made translatable; however, since this only contains a fixed command example which references the man pages, we can avoid translating it.

tq/api_test.go: remove unneeded translations

816857e

In commit 08774bf of PR git-lfs#4781 two message strings in the tq/api_test.go file were made translatable; however, since this only contains Go tests, we can reverse that change.

chrisd8088 added 5 commits January 29, 2022 22:32

chrisd8088 requested review from a team and bk2204 and removed request for bk2204 January 30, 2022 08:18

bk2204 approved these changes Jan 31, 2022

View reviewed changes

commands/command_filter_process.go Outdated Show resolved Hide resolved

chrisd8088 added 2 commits January 31, 2022 11:39

commands/command_filter_process.go: fix message

81b6faf

Now that we have quantity-sensitive text messages for translation purposes, we can correct one message to use the singular form. h/t bk2204 on PR review.

Merge branch 'main' into extra-git-string-translations

a483a87

chrisd8088 merged commit 12a9572 into git-lfs:main Jan 31, 2022

chrisd8088 deleted the extra-git-string-translations branch January 31, 2022 20:47

chrisd8088 mentioned this pull request Apr 14, 2022

Update and correct several error message strings #4943

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update and mark message strings for translation#4846

Update and mark message strings for translation#4846
chrisd8088 merged 37 commits intogit-lfs:mainfrom
chrisd8088:extra-git-string-translations

chrisd8088 commented Jan 30, 2022 •

edited

Loading

Uh oh!

bk2204 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

chrisd8088 commented Jan 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bk2204 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chrisd8088 commented Jan 30, 2022 •

edited

Loading