Add more time-based variables for snippets#46049
Merged
jrieken merged 3 commits intomicrosoft:masterfrom Mar 20, 2018
usernamehw:additional_time_variables
Merged
Add more time-based variables for snippets#46049jrieken merged 3 commits intomicrosoft:masterfrom usernamehw:additional_time_variables
jrieken merged 3 commits intomicrosoft:masterfrom
usernamehw:additional_time_variables
Conversation
Member
We want it now. Please use |
Contributor
Author
|
Are we talking about changing already defined ones or adding 4 new variables? There might be cases in which someone with localized version would want to show the default (English). |
Member
Those defined in this PR, also don't substring the long variants but have them separately as short versions. |
jrieken
requested changes
Mar 20, 2018
|
|
||
| resolve(variable: Variable): string { | ||
| const { name } = variable; | ||
| const dayNames = [nls.localize('Sunday', "Sunday"), nls.localize('Monday', "Monday"), nls.localize('Tuesday', "Tuesday"), nls.localize('Wednesday', "Wednesday"), nls.localize('Thursday', "Thursday"), nls.localize('Friday', "Friday"), nls.localize('Saturday', "Saturday")]; |
Member
There was a problem hiding this comment.
It would be better to keep those as static readonly propoerties because we don't need to recompute those string-arrays all the time. Otherwise looking good.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #43140
Not 100% sure about short months:
SepvsSept. They currently all 3-letter abbreviations and can be easily aligned:If it's critical it can be changed to
Sept. In that caseJun&Julwould beJune&July, I guess...If someone wants localized versions -
New issuebutton awaits.