Skip to content

Fix #19959 and #19958: Remove un-localizable messages #20019

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

Merged
merged 4 commits into from
Nov 25, 2017

Conversation

mhegazy
Copy link
Contributor

@mhegazy mhegazy commented Nov 14, 2017

Fixes #19959
Fixes #19958

@weswigham
Copy link
Member

If Import {0} from {1} is supposed to refer to the syntax (and it looks like it), then that should also be unlocalizable, I imagine? Would it not be more clear to use a Replace with '{0}' and\or Insert '{0}', where 0 is the entire printed replacement node tree? That moves all syntaxish bits into a quote and leaves "replace with" or "insert" as localizable.

@mhegazy
Copy link
Contributor Author

mhegazy commented Nov 16, 2017

If Import {0} from {1} is supposed to refer to the syntax (and it looks like it), then that should also be unlocalizable, I imagine? Would it not be more clear to use a Replace with '{0}' and\or Insert '{0}', where 0 is the entire printed replacement node tree? That moves all syntaxish bits into a quote and leaves "replace with" or "insert" as localizable.

No, it just says import "foo" from module "bar", like the English text for "should i import this for you". Replace is not accurate, since the import was not there int he first place. insert seems redundant, like Insert import for "foo" from "bar", why not just import?

@mhegazy
Copy link
Contributor Author

mhegazy commented Nov 23, 2017

import still seems like a good action verb to use here.. i have added module to the message to ensure it looks different from the syntax of the import to avoid confusion.

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

Seems ok once all the test baselines are updated.

@mhegazy mhegazy merged commit 6b3cfc7 into master Nov 25, 2017
@mhegazy mhegazy deleted the FixLocalizabilityForImportMessages branch November 25, 2017 06:02
errendir added a commit to errendir/TypeScript that referenced this pull request Nov 27, 2017
* origin/master:
  LEGO: check in for master to temporary branch.
  Fix microsoft#19959 and microsoft#19958: Remove un-localizable messages (microsoft#20019)
  LEGO: check in for master to temporary branch.
  LEGO: check in for master to temporary branch.
  LEGO: check in for master to temporary branch.
  LEGO: check in for master to temporary branch.
  Port generated lib files (microsoft#20213)
  Update test baseline
  Offer global completions in all blocks
  Accept new baselines
  Add regression test
  Make sure 'async' isn't treated as a parameter modifier
@microsoft microsoft locked and limited conversation to collaborators Jun 14, 2018
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.

2 participants