Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reword or remove a bunch of subheadings in the SUI #15362

Merged
merged 3 commits into from
May 16, 2023

Conversation

DHowett
Copy link
Member

@DHowett DHowett commented May 15, 2023

Some of these were reundant, and some didn't feel right when I read them.

Oh, and I got rid of all of these particularly unhelpful or non-additive resources:

Color Scheme        [                     v ]
Is a color scheme

The only one I am truly iffy about is the one that newly says OSC 2.

@@ -987,8 +971,8 @@
<comment>Header for a control to toggle classic CRT display effects, which gives the terminal a retro look.</comment>
</data>
<data name="Profile_RetroTerminalEffect.HelpText" xml:space="preserve">
<value>When enabled, enables retro terminal effects such as glowing text and scan lines.</value>
Copy link
Member Author

Choose a reason for hiding this comment

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

When enabled, enables.

<value>Sets an override for the app's preferred language.</value>
<comment>A description explaining how this control changes the app's language.</comment>
<value>Selects a display language for the application. This will override your default Windows interface language.</value>
<comment>A description explaining how this control changes the app's language. {Locked="Windows"}</comment>
Copy link
Member

Choose a reason for hiding this comment

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

I'm not 100% if Windows is "Windows" in all languages. The Hindi Wikipedia article for instance is माइक्रोसॉफ्ट विंडोज़ ("maikrosopht vindoz").

Copy link
Member Author

Choose a reason for hiding this comment

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

fair, but I think we have locked Windows elsewhere in the code. hmm.

@@ -895,7 +879,7 @@
<comment>Header for a control that sets the text line height.</comment>
</data>
<data name="Profile_LineHeight.HelpText" xml:space="preserve">
<value>Sets the height of each line in the terminal as a multiple of the font size. The default depends on your font and is usually around 1.2.</value>
<value>Override the height of a line of text in the terminal. Measured as a multiple of the font size. The default value depends on your font, and is usually around 1.2.</value>
Copy link
Member

Choose a reason for hiding this comment

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

"a line"? There are many lines. 🤔 Given CSS and Word, I think "line height" as a term is fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

but it also doesn't set the height of each line, it sets the height of all lines. hmm

Copy link
Member Author

Choose a reason for hiding this comment

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

MDN calls it "a line box"!
image

Copy link
Member

Choose a reason for hiding this comment

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

but it also doesn't set the height of each line, it sets the height of all lines. hmm

What's the difference between those? 😵‍💫

Copy link
Member Author

Choose a reason for hiding this comment

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

It's not a language thing, it's more of a feel thing. "Setting the height of each line" suggests to me that you could say "line 1 is this tall, line 2 however is THAT tall, etc etc".

It's very minor, and it is probably ENTIRELY in my head :D

Copy link
Member

Choose a reason for hiding this comment

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

Oh, you meant the old text. I had a simpler form in mind:

Sets the line height of the terminal as a multiple of the font size.

Or, merged with your PR:

Override the line height of the terminal.

Copy link
Member Author

Choose a reason for hiding this comment

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

OH. Yeah, totally. Thanks XD

@@ -987,8 +971,8 @@
<comment>Header for a control to toggle classic CRT display effects, which gives the terminal a retro look.</comment>
</data>
<data name="Profile_RetroTerminalEffect.HelpText" xml:space="preserve">
<value>When enabled, enables retro terminal effects such as glowing text and scan lines.</value>
<comment>A description for what the "retro terminal effects" setting does. Presented near "Profile_RetroTerminalEffect".</comment>
<value>Show "retro" terminal effects such as glowing text and scan lines.</value>
Copy link
Member

Choose a reason for hiding this comment

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

I think "retro-style" would be better than quoting the word "retro". It also seems like this translates better into other languages (judging by Google Translate).

Copy link
Member Author

Choose a reason for hiding this comment

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

good call

Copy link
Contributor

@PankajBhojwani PankajBhojwani left a comment

Choose a reason for hiding this comment

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

Just a nit, looks good otherwise

@@ -1055,8 +1039,8 @@
<comment>Header for a control to toggle changes in the app title.</comment>
</data>
<data name="Profile_SuppressApplicationTitle.HelpText" xml:space="preserve">
<value>Use the tab title to override the default title of the tab and suppress any title change messages from the application.</value>
<comment>A description for what the "suppress application title" setting does. Presented near "Profile_SuppressApplicationTitle".</comment>
<value>Ignore application requests to change the title using OSC 2.</value>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can add a little more here like "using the OSC 2 Virtual Terminal Sequence" so the user has something to work with if they don't understand what "OSC 2" means.

Copy link
Member Author

Choose a reason for hiding this comment

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

that may be harder to translate, and there is not a hugely accepted vocabulary about what these are actually called. Technically, "in-band control sequences"... but also OSC is "operating system control", so the "operating system control control sequence". But also, it doesn't use CSI - which is the "Control Sequence Introducer"? It's complicated!

Copy link
Member Author

Choose a reason for hiding this comment

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

idk. i'm open to it!

Copy link
Member

@lhecker lhecker May 16, 2023

Choose a reason for hiding this comment

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

I'd personally write

Ignore application requests to change the title (OSC 2).

I feel like saying that it's a VT sequence or only OSC 2 doesn't really add anything to the idea of "prevent title changes". Especially since SetWindowText is not OSC 2 - it just so happens that we translate it to OSC 2 in ConPTY which is sort of an implementation detail of WT from a user perspective. 🤔 I would personally only add the "OSC 2" in parenthesis at the end, because it might help with google-ing it.

Copy link
Contributor

@PankajBhojwani PankajBhojwani May 16, 2023

Choose a reason for hiding this comment

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

Not saying anything related to VT sequences doesn't help with looking it up online - try searching for "OSC 2" and see the results. That's why I'd like some sort of extra explanation/context so users can try to search for what it means

Copy link
Member Author

Choose a reason for hiding this comment

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

To be fair, on its own it doesn't find much... but terminal osc 2 sure does.

@DHowett DHowett merged commit 62766db into main May 16, 2023
@DHowett DHowett deleted the dev/duhowett/resw-rewords branch May 16, 2023 19:59
DHowett added a commit that referenced this pull request May 16, 2023
Some of these were reundant, and some didn't feel right when I read
them.

Oh, and I got rid of all of these particularly unhelpful or non-additive
resources:

```
Color Scheme        [                     v ]
Is a color scheme
```

(cherry picked from commit 62766db)
Service-Card-Id: 89230303
Service-Version: 1.17
DHowett added a commit that referenced this pull request May 16, 2023
Some of these were reundant, and some didn't feel right when I read
them.

Oh, and I got rid of all of these particularly unhelpful or non-additive
resources:

```
Color Scheme        [                     v ]
Is a color scheme
```

(cherry picked from commit 62766db)
Service-Card-Id: 89230304
Service-Version: 1.18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants