Skip to content

Conversation

@florelis
Copy link
Member

@florelis florelis commented Mar 7, 2025

Off-shot of @pressRtowin's #5279

This adds periods at the end of all string resources for argument descriptions and error codes. It also replaces a bunch of semicolons by periods.

Microsoft Reviewers: Open in CodeFlow

@florelis florelis requested a review from a team as a code owner March 7, 2025 20:09
@pressRtowin
Copy link
Contributor

pressRtowin commented Mar 7, 2025

Some notes:

  • Is it any more correct to add periods to lines that aren't technically sentences?
  • Also some lines with fixed semicolons seem to have not received their final periods.
  • There's a lot of inconsistent use of the imperative vs. 3rd person with a pronoun drop, and technically only the former is proper English (and therefore a "sentence"), i.e. "Installs a thingy." is not correct, as English is not a pro-drop language except for the imperative mood and requires a subject here ("This installs a thingy."). However, if it were re-written to "Install a thingy.", that's perfectly fine, and many other lines are written this way.
  • As long as we're getting pedantic about grammar, there's also very inconsistent dropping of articles, even within the same sentence. "The path to the manifest of the package." elects to use every article, but "Override arguments to be passed on to the installer." drops the article for "arguments" ("the"), but doesn't for "installer".

I started off making a list of line numbers with issues, until I realized how much of a massive waste of time it was due to how many instances like this there are, and time would be better spent just directly fixing them.

@yao-msft
Copy link
Contributor

yao-msft commented Mar 7, 2025

Some of the strings are command/argument description. I thought we didn't include period on purpose?
image

Do we want to revisit the strings based on use cases?

@florelis
Copy link
Member Author

florelis commented Mar 7, 2025

Some of the strings are command/argument description. I thought we didn't include period on purpose? image

Do we want to revisit the strings based on use cases?

I did specifically change the argument descriptions and error messages. Currently most don't have periods, but some do. I just want to make it consistent in either direction. If what we want is for them to not have periods, that's cool by me.

I went to the side of yes periods because some strings had two sentences and it felt weird to only have the first one have a period in the first one. For example, List packages even if their current version cannot be determined. Can only be used with the --upgrade-available argument. Admittedly, all those seem to be my fault and could probably be reworded to be a single sentence 😅

I also wondered if there was a standard for CLI tools and noticed that both powershell -? and dotnet -? use periods

@pressRtowin
Copy link
Contributor

pressRtowin commented Mar 7, 2025

Some of the strings are command/argument description. I thought we didn't include period on purpose? image

Do we want to revisit the strings based on use cases?

A few examples of my 3rd point in that screenshot. Most are imperative ("Filter", "Use", "Find", "Select", etc.) but then why "Allows" and "Skips"?

And my 4th point too I guess:

  • "Find package dependencies using the specified source" instead of "Find a package's dependencies using the specified source"
  • "Skips upgrade if an installed version already exists" instead of "Skips the upgrade if an installed version already exists", or well I guess "Skip the upgrade if an installed version already exists" since point 3.

@florelis
Copy link
Member Author

florelis commented Mar 7, 2025

  • Is it any more correct to add periods to lines that aren't technically sentences?

Maybe it's not more grammatically correct, but it looks prettier if it's consistent and we can keep iterating on the grammar :D

  • Also some lines with fixed semicolons seem to have not received their final periods.

Fixed

  • There's a lot of inconsistent use of the imperative vs. 3rd person with a pronoun drop, and technically only the former is proper English (and therefore a "sentence"), i.e. "Installs a thingy." is not correct, as English is not a pro-drop language except for the imperative mood and requires a subject here ("This installs a thingy.").

I'm not educated enough in proper grammar to weigh in on this if it's going to need a larger change across many strings. I'd need either references or someone else to confirm.

Most are imperative ("Filter", "Use", "Find", "Select", etc.) but then why "Allows" and "Skips"?

For the "why": Because these are written over time by different people and that makes it easy for inconsistencies to develop. It's always nice to have someone look at it all together to point these things out :)

Making the tense consistent is not as simple/quick as a search-and-replace, so I probably won't include it in this PR

@pressRtowin
Copy link
Contributor

pressRtowin commented Mar 7, 2025

I'm not educated enough in proper grammar to weigh in on this if it's going to need a larger change across many strings. I'd need either references or someone else to confirm.

Basically if a sentence is dropping the subject, the only case where it's technically appropriate to do so (in English) is in what's called the "imperative mood", which generally takes the form of a command, for example, sentences like "Run!" or "Sit down.". These are always in second person, and the "You" is implied, e.g. "You sit down.". By using the 3rd person singular form of a verb, you're establishing that it's not in the imperative mood, e.g. "Allows a reboot if applicable.". This isn't seen as valid sentence structuring in English as the subject being dropped is 3rd-person, not 2nd. The full sentence being implied here is "This allows a reboot if applicable.", and that's not a case where it's acceptable to drop the subject in English.

However, this is done quite frequently in "documentation" type settings, which is why I stated in the other PR that it's basically a philosophical debate at this point, as it's widely done but technically incorrect.

Either way, it should go one way or the other.

For the "why": Because these are written over time by different people and that makes it easy for inconsistencies to develop. It's always nice to have someone look at it all together to point these things out :)

Oh, I know. My "why" was very rhetorical haha.

I'd need some time, but I could definitely go through that whole document and make a definitive "Pedant's Edit" at some point in the near future lol

@denelon
Copy link
Collaborator

denelon commented Mar 7, 2025

@pressRtowin I didn't see the other PR you made.

THANK YOU SO MUCH!

There are so many cases where we've failed to be consistent. It's been a challenge to prioritize cleaning up the mess. I really appreciate the time taken to review/improve the situation.

I was just talking with another PM this afternoon about the UX for the WinGet CLI. I'm going to point her at this PR as well just for the sake of keeping her informed.

I don't want to introduce any breaking changes, but the other area I've struggled with is in the naming of the commands. Most of them are verbs like "install" and "upgrade". Some of them are nouns like "settings" and "features". A couple of them could be ambiguous like "hash" and "pin".

@Trenly
Copy link
Contributor

Trenly commented Mar 7, 2025

I'd also like to chime in, especially with regards to this specific thought -

As long as we're getting pedantic about grammar, there's also very inconsistent dropping of articles, even within the same sentence. "The path to the manifest of the package." elects to use every article, but "Override arguments to be passed on to the installer." drops the article for "arguments" ("the"), but doesn't for "installer".

In some cases, and the example here is a great one, the intent is truly that the object of the sentence is the plural arguments where Override is an adjective describing the type of arguments, and in these cases the form Override arguments is just as correct as Override the arguments. It is just a matter of whether what is being described is an action or an input

@pressRtowin
Copy link
Contributor

@pressRtowin I didn't see the other PR you made.

THANK YOU SO MUCH!

There are so many cases where we've failed to be consistent. It's been a challenge to prioritize cleaning up the mess. I really appreciate the time taken to review/improve the situation.

I was just talking with another PM this afternoon about the UX for the WinGet CLI. I'm going to point her at this PR as well just for the sake of keeping her informed.

I don't want to introduce any breaking changes, but the other area I've struggled with is in the naming of the commands. Most of them are verbs like "install" and "upgrade". Some of them are nouns like "settings" and "features". A couple of them could be ambiguous like "hash" and "pin".

Glad I could help! And yeah, consistency with parts of speech like that can really help especially if you're not familiar with the tool and trying to guess/remember commands without referring to the docs over and over.

In some cases, and the example here is a great one, the intent is truly that the object of the sentence is the plural arguments where Override is an adjective describing the type of arguments, and in these cases the form Override arguments is just as correct as Override the arguments. It is just a matter of whether what is being described is an action or an input

I had assumed "Override" was a verb here, as many of the other descriptors start with a verb, but yes, since "Override arguments" is functioning as a noun phrase here, it's fine (though maybe rewording it for clarity might be warranted?). There are other examples of the issue I was describing though, such as "Unable to create (the/a) symlink. (The) Path points to a directory.", "Specify (the/a) authentication window preference (silent, silentPreferred, or interactive)", and "Failed to add (the/a) source. This winget version does not support the source's authentication method. Try upgrading to latest (the) winget version.", etc.

@pressRtowin
Copy link
Contributor

I also wondered if there was a standard for CLI tools and noticed that both powershell -? and dotnet -? use periods

Thank you for mentioning powershell -? by the way. Thanks to you, I now have a PR on the Powershell repo to add one comma 🤣

@florelis
Copy link
Member Author

Talked with @JohnMcPMS offline. For argument descriptions the main priority is to keep them short since the terminal has a limited space, even if it is not perfectly grammatical. I'm leaving argument descriptions with no periods as that's what most already had. For consistency, I removed periods from the few arguments that had it, and changed the few ones that were two "sentences" to be only one.

For the errors it makes sense to me to have them be more proper English, plus it aligns with other Windows error codes.

</data>
<data name="NoApplicableInstallers" xml:space="preserve">
<value>No applicable installer found; see logs for more details.</value>
<value>No applicable installer found. See logs for more details.</value>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be "No applicable installer was found."?

</data>
<data name="PortableRegistryCollisionOverridden" xml:space="preserve">
<value>A portable package with the same name but from a different source already exists; proceeding due to --force</value>
<value>A portable package with the same name but from a different source already exists. Proceeding due to --force.</value>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, "Proceeding due to --force." is a fragment (no subject), but I'm not sure what the best way to fix it would be. Something like "The process will continue due to --force." maybe?

</data>
<data name="SourceOpenPredefinedFailedSuggestion" xml:space="preserve">
<value>Failed to open the predefined source; please report to winget maintainers.</value>
<value>Failed to open the predefined source. Please report to winget maintainers.</value>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as for line 601.

Additionally, when "report" is used as an intransitive verb, it would mean "to check in" as in "report to the office". So unless we want people just checking in with the winget team, we probably want the transitive "report" (to submit an account of something), so an object is required (with another minor tweak as well): "Please report this to winget's maintainers."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed it to say "winget's maintainers" but the style guide says

Note Don't use the possessive form of Microsoft trademarks and product, service, or feature names.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note Don't use the possessive form of Microsoft trademarks and product, service, or feature names.

"Please report this to those who maintain winget."? 😅

" . . . to the maintainers of winget." might work. Adheres to the guide without sounding biblical.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just laughed out loud and almost ended up wearing my coffee.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose this should be "WinGet" then rather than "winget" as it's referring to the maintainers of the product, not the command itself.

</data>
<data name="PortableInstallFailed" xml:space="preserve">
<value>Portable install failed; Cleaning up...</value>
<value>Portable install failed. Cleaning up...</value>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The portable installation failed." probably? "Cleaning up." is also technically a problem but I have no clue what to replace that with.

</data>
<data name="PortableHashMismatchOverridden" xml:space="preserve">
<value>Portable package has been modified; proceeding due to --force</value>
<value>Portable package has been modified. Proceeding due to --force.</value>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The portable package . . . "

Also, as with line 1394, "The process will continue due to --force." may be preferable.

<value>No package selection argument was provided. See the help for details about finding a package.</value>
</data>
<data name="PortableAliasAdded" xml:space="preserve">
<value>Command line alias added:</value>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wasn't edited here, but does this fall under the scope? If so, "A command line alias was added" probably?

</data>
<data name="PortableHashMismatchOverrideRequired" xml:space="preserve">
<value>Unable to remove Portable package as it has been modified; to override this check use --force</value>
<value>Unable to remove Portable package as it has been modified. To override this check use --force.</value>
Copy link
Contributor

@pressRtowin pressRtowin Mar 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The portable package cannot be removed as it has been modified. To override this check*,* use --force."

</data>
<data name="InvalidPathToNestedInstaller" xml:space="preserve">
<value>Invalid relative file path to the nested installer; path points to a location outside of the install directory</value>
<value>Invalid relative file path to the nested installer. Path points to a location outside of the install directory.</value>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The relative file path to the nested installer is invalid. The path points to . . . "

@denelon
Copy link
Collaborator

denelon commented Mar 12, 2025

It's been a while for me (I remember the Microsoft Manual of Style), but it looks like the latest "writing" guidance for US English is https://learn.microsoft.com/en-us/style-guide/welcome/

@pressRtowin
Copy link
Contributor

It's been a while for me (I remember the Microsoft Manual of Style), but it looks like the latest "writing" guidance for US English is https://learn.microsoft.com/en-us/style-guide/welcome/

I checked out this page: https://learn.microsoft.com/en-us/style-guide/grammar/verbs

Unfortunately it's not as helpful as I had been hoping it'd be for determining when to use the indicative vs the imperative. I don't know if the most recent proposed changes addressed any of this, but there was a lot of mix between the two, especially when describing options and parameters, as I mentioned before.

For what it's worth, PowerShell is also quite inconsistent with this, so looking to them for guidance doesn't exactly help either. (-Sta "Starts the shell using a single-threaded apartment. vs -Mta Start the shell using a multithreaded apartment.

Then again, they do use periods for all those, and it seems we just elected not to, so I'm not sure how much consistency between Microsoft products we're aiming for.

@pressRtowin
Copy link
Contributor

AHA! Found it!

https://learn.microsoft.com/en-us/style-guide/developer-content/formatting-developer-text-elements

Under "UI text or strings", the provided examples are all in the imperative:

  • Import from file (though I'd argue this should be "Import from a file"
  • Create a new resource
  • See all your resources
  • Manually trigger a flow
  • Report a bug

They also don't use periods here (despite them being proper sentences due to the imperative mood), but they don't actually explicitly say to use or not use periods anywhere on that chart, despite examples given in other categories (such as for error messages) all consistently either using or not using periods for the whole category.

@florelis florelis changed the title Add all the periods Make the strings use gooder English Mar 12, 2025
@florelis florelis marked this pull request as draft March 12, 2025 21:36
@florelis
Copy link
Member Author

Marking this as a draft because clearly there are still a lot of strings to fix 😅

@denelon
Copy link
Collaborator

denelon commented Mar 12, 2025

My 2 cents: As long as we're consistent with "ourselves" in WinGet I think we're heading in the right direction. I'd like to be a great example for others to follow (assuming we're doing it the right way and honoring the guidance given). Given the problems we have with terminal width and how word wrapping is implemented, the main reason to omit trailing periods is to avoid an unwanted line feed.

@pressRtowin
Copy link
Contributor

My 2 cents: As long as we're consistent with "ourselves" in WinGet I think we're heading in the right direction. I'd like to be a great example for others to follow (assuming we're doing it the right way and honoring the guidance given). Given the problems we have with terminal width and how word wrapping is implemented, the main reason to omit trailing periods is to avoid an unwanted line feed.

But I love reading text like
this.

But yeah that makes sense, and seems to be in line with what the guide (silently) suggests.

On that note though, is it "winget" or "WinGet"?

</data>
<data name="InvalidPathToNestedInstaller" xml:space="preserve">
<value>Invalid relative file path to the nested installer; path points to a location outside of the install directory</value>
<value>Invalid relative file path to the nested installer. The path points to a location outside of the install directory.</value>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"The relative file path to the nested installer is invalid."

@denelon
Copy link
Collaborator

denelon commented Mar 13, 2025

"WinGet" is the product name. There are some scenarios where the lower case makes sense in terms of one referencing the "winget" command. It depends on whether one is referring to the product or the executable. It's also valid to have a reference to the WinGet CLI (Command Line Interface). <- Yes I have a hard time using WinGet Command Line Interface (CLI) properly too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants