Skip to content

Change label of indentation action in status bar#37515

Merged
alexdima merged 2 commits intomicrosoft:masterfrom
waldyrious:wp/select-indentation
Mar 26, 2018
Merged

Change label of indentation action in status bar#37515
alexdima merged 2 commits intomicrosoft:masterfrom
waldyrious:wp/select-indentation

Conversation

@waldyrious
Copy link
Contributor

@waldyrious waldyrious commented Nov 2, 2017

This change is to conform to the labels of the other actions, which clearly indicate that they will provide a way to do something ("Go to", "Select", etc.)

I separated the changes into multiple atomic commits, so that if any of the changes are not acceptable (e.g. changes to the localization keys, or localization updates that don't go through Transifex), they can be easily left out.

This change is to conform to the labels of the other actions,
which clearly indicate that they will provide a way to *do* something
(Go to, Select, etc.)
@octref octref requested a review from dbaeumer November 2, 2017 22:35
@dbaeumer dbaeumer assigned danyeh and unassigned dbaeumer Nov 3, 2017
@dbaeumer
Copy link
Member

dbaeumer commented Nov 3, 2017

@waldyrious as noted in the header of these files they are machine generated.

The way to contribute to translations is via Transifex. Please see https://github.com/Microsoft/Localization/wiki/Visual-Studio-Code-Community-Localization-Project

@waldyrious
Copy link
Contributor Author

@dbaeumer should I drop bc152db and 0b583f6, then? And what about the key change -- is that OK?

@dbaeumer
Copy link
Member

dbaeumer commented Nov 3, 2017

@waldyrious the keys are in our source code and need to be changed there . The translation is managed in transifex and need to be changed there.

For example src/vs/workbench/browser/parts/editor/editorStatus.ts for the corresponding file you changed in i18n.

@waldyrious
Copy link
Contributor Author

waldyrious commented Nov 3, 2017

@dbaeumer just to be sure, are you saying that commit 5876213 can remain in this PR?

@dbaeumer
Copy link
Member

dbaeumer commented Nov 3, 2017

@waldyrious
Copy link
Contributor Author

No, For example the first change has to happen here: https://github.com/Microsoft/vscode/blob/master/src/vs/workbench/browser/parts/editor/editorStatus.ts#L308

@dbaeumer 5876213 does change that (here). Are you saying that that change must happen in isolation (i.e. that 5876213 should only contain the change to editorStatus.ts), and the key will automatically be updated in the i18n files after the PR is merged?

@dbaeumer
Copy link
Member

dbaeumer commented Nov 3, 2017

The key change has to happen in our source code. Then in transifex the message (since it is still the same will be recycled) and when we import the next time from transfix the key change will appear in the i18n files. Message changes have to appear in Transifex itself.

What is the purpose of the key change? Usually we only change them if absolutely necessary.

@waldyrious
Copy link
Contributor Author

The key change has to happen in our source code. Then in transifex the message (since it is still the same will be recycled) and when we import the next time from transfix the key change will appear in the i18n files. Message changes have to appear in Transifex itself.

Got it, thanks for the confirmation. I'll update the branch accordingly.

What is the purpose of the key change? Usually we only change them if absolutely necessary.

The change in the key is to keep it consistent with both the message contents and the other keys:

-indentation --> "Indentation"
+selectIndentation --> "Select Indentation"
 selectEncoding --> "Select Encoding"
 selectEOL --> "Select End of Line Sequence"
 selectLanguageMode --> "Select Language Mode"

If I changed only the message contents, then the key name would be inconsistent and potentially confusing.

This is to match both its contents and the keys of similar actions in the status bar
@waldyrious waldyrious force-pushed the wp/select-indentation branch from 0b583f6 to 32357e9 Compare November 3, 2017 17:59
@danyeh danyeh assigned dbaeumer and unassigned danyeh Jan 25, 2018
@danyeh
Copy link
Contributor

danyeh commented Jan 25, 2018

@dbaeumer , it is code change. So assigned back to you.

@dbaeumer
Copy link
Member

This is editor. Assigning to @alexandrudima

@dbaeumer dbaeumer assigned alexdima and unassigned dbaeumer Jan 26, 2018
@alexdima alexdima added this to the February 2018 milestone Feb 1, 2018
@bpasero bpasero modified the milestones: February 2018, March 2018 Mar 8, 2018
@alexdima alexdima merged commit 0eb68ef into microsoft:master Mar 26, 2018
@waldyrious waldyrious deleted the wp/select-indentation branch March 26, 2018 14:34
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants