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

Change label of indentation action in status bar #37515

Merged
merged 2 commits into from Mar 26, 2018

Conversation

Projects
None yet
5 participants
@waldyrious
Copy link
Contributor

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.

change label of indentation action in status bar
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 Nov 2, 2017

@dbaeumer dbaeumer assigned danyeh and unassigned dbaeumer Nov 3, 2017

@dbaeumer

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

waldyrious commented Nov 3, 2017

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

@dbaeumer

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

waldyrious commented Nov 3, 2017

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

@dbaeumer

This comment has been minimized.

Copy link
Member

dbaeumer commented Nov 3, 2017

@waldyrious

This comment has been minimized.

Copy link
Contributor

waldyrious commented Nov 3, 2017

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Contributor

waldyrious 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.

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.

change l10n key of status bar's Indentation action
This is to match both its contents and the keys of similar actions in the status bar

@waldyrious waldyrious force-pushed the waldyrious:wp/select-indentation branch from 0b583f6 to 32357e9 Nov 3, 2017

@danyeh danyeh assigned dbaeumer and unassigned danyeh Jan 25, 2018

@danyeh

This comment has been minimized.

Copy link
Collaborator

danyeh commented Jan 25, 2018

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

@dbaeumer

This comment has been minimized.

Copy link
Member

dbaeumer commented Jan 26, 2018

This is editor. Assigning to @alexandrudima

@dbaeumer dbaeumer assigned alexandrudima and unassigned dbaeumer Jan 26, 2018

@alexandrudima alexandrudima added this to the February 2018 milestone Feb 1, 2018

@bpasero bpasero modified the milestones: February 2018, March 2018 Mar 8, 2018

@alexandrudima alexandrudima merged commit 0eb68ef into Microsoft:master Mar 26, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
license/cla All CLA requirements met.

@waldyrious waldyrious deleted the waldyrious:wp/select-indentation branch Mar 26, 2018

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