fix: to #157015 fix view label command localized by yiliang114 · Pull Request #193544 · microsoft/vscode

yiliang114 · 2023-09-20T04:51:25Z

TylerLeonhardt · 2023-09-21T18:14:59Z

Awesome!

Looks like you do have a couple of issues still:

[tsec-compile-check      ] src/vs/workbench/contrib/scm/browser/scm.contribution.ts(115,2): error TS2322: Type 'string' is not assignable to type 'ILocalizedString'.
[tsec-compile-check      ] src/vs/workbench/contrib/userDataSync/browser/userDataSyncViews.ts(181,32): error TS2352: Conversion of type '{ id: string; name: string; ctorDescriptor: SyncDescriptor<TreeViewPane>; when: RawContextKey<boolean>; canToggleVisibility: true; canMoveView: false; treeView: TreeView; collapsed: false; hideByDefault: false; }' to type 'ITreeViewDescriptor' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
[tsec-compile-check      ]   Types of property 'name' are incompatible.
[tsec-compile-check      ]     Type 'string' is not comparable to type 'ILocalizedString'.
[tsec-compile-check      ] 
[tsec-compile-check      ] 
[tsec-compile-check      ] Found 2 errors.
[tsec-compile-check      ] error Command failed with exit code 1.

From the CI

src/vs/workbench/contrib/quickaccess/browser/viewQuickAccess.ts

yiliang114 · 2023-09-22T02:43:33Z

Awesome!

Looks like you do have a couple of issues still:

[tsec-compile-check      ] src/vs/workbench/contrib/scm/browser/scm.contribution.ts(115,2): error TS2322: Type 'string' is not assignable to type 'ILocalizedString'.
[tsec-compile-check      ] src/vs/workbench/contrib/userDataSync/browser/userDataSyncViews.ts(181,32): error TS2352: Conversion of type '{ id: string; name: string; ctorDescriptor: SyncDescriptor<TreeViewPane>; when: RawContextKey<boolean>; canToggleVisibility: true; canMoveView: false; treeView: TreeView; collapsed: false; hideByDefault: false; }' to type 'ITreeViewDescriptor' may be a mistake because neither type sufficiently overlaps with the other. If this was intentional, convert the expression to 'unknown' first.
[tsec-compile-check      ]   Types of property 'name' are incompatible.
[tsec-compile-check      ]     Type 'string' is not comparable to type 'ILocalizedString'.
[tsec-compile-check      ] 
[tsec-compile-check      ] 
[tsec-compile-check      ] Found 2 errors.
[tsec-compile-check      ] error Command failed with exit code 1.

From the CI

Thanks!

Now it is reasonable to say that all the processing has been completed.

(I think I should have handled all the compilation errors before. When I update the main branch code, some code may be restored due to automatic merging)

src/vs/workbench/contrib/editSessions/browser/editSessionsViews.ts

src/vs/workbench/contrib/userDataSync/browser/userDataSyncViews.ts

src/vs/workbench/services/userDataProfile/browser/userDataProfileImportExportService.ts

TylerLeonhardt

This is super close! Just a few comments

Copilot-based TS refactors

…deer add workspace tags

add comment help info to diff help dialog

…t-code-mappers-context support getting various document ranges from InteractiveSessionProviders

src/vs/workbench/browser/parts/views/viewsService.ts

TylerLeonhardt · 2023-09-27T22:04:16Z

FYI we are currently in the middle of a release right now, so I want to hold off on getting this in til next week. That way we can let it sit in Insiders for a bit to make sure we don't cause any regressions due to the number of files changed. I think the PR is ready, but I'll approve and merge early next week.

src/vs/workbench/services/views/test/browser/viewDescriptorService.test.ts

TylerLeonhardt · 2023-10-04T21:21:27Z

@yiliang114 I hope you don't hate me for this but I just added a new thing that will make this PR much smaller and have less duplicate strings.

I have added a localize2 function in this PR that allows us to replace all of these:

{ value: nls.localize({ comment: ['Debug is a noun in this context, not a verb.'], key: 'debugPanel' }, "Debug Console"), original: 'Debug Console' }

with a much simpler:

nls.localize2({ comment: ['Debug is a noun in this context, not a verb.'], key: 'debugPanel' }, "Debug Console")

this function returns an ILocalizedString and handles the original string correctly.

Can you move over to using that so the code is easier to maintain and we don't need to have two of the same string next to each other?

I think this won't apply to everything in your PR but probably about 80-90% if the cases.

yiliang114 · 2023-10-07T05:33:19Z

@yiliang114 I hope you don't hate me for this but I just added a new thing that will make this PR much smaller and have less duplicate strings.

I have added a localize2 function in this PR that allows us to replace all of these:
{ value: nls.localize({ comment: ['Debug is a noun in this context, not a verb.'], key: 'debugPanel' }, "Debug Console"), original: 'Debug Console' }
with a much simpler:
nls.localize2({ comment: ['Debug is a noun in this context, not a verb.'], key: 'debugPanel' }, "Debug Console")
this function returns an ILocalizedString and handles the original string correctly.

Can you move over to using that so the code is easier to maintain and we don't need to have two of the same string next to each other?

I think this won't apply to everything in your PR but probably about 80-90% if the cases.

Haha, it looks better this way. Let me try again.

…t-code-mappers-context support getting various document ranges from InteractiveSessionProviders

yiliang114 · 2023-10-09T03:20:57Z

Please help me review the code again when you are free @TylerLeonhardt , I did not encountered an error when I compiled locally.

TylerLeonhardt

LGTM thanks for taking this on!

roblourens · 2023-10-10T03:11:04Z

src/vs/workbench/services/views/test/browser/viewContainerModel.test.ts


 	test('empty model', function () {
-		container = ViewContainerRegistry.registerViewContainer({ id: 'test', title: { value: 'test', original: 'test' }, ctorDescriptor: new SyncDescriptor(<any>{}) }, ViewContainerLocation.Sidebar);
+		container = ViewContainerRegistry.registerViewContainer({ id: 'test', title: nls.localize2('test', 'test'), ctorDescriptor: new SyncDescriptor(<any>{}) }, ViewContainerLocation.Sidebar);


These will be excluded from the actual localization process, right?

…tring` Fixes #195518 regressed in #193544

…tring` (#195709) Fixes #195518 regressed in #193544

…193544) * fix: to microsoft#157015 fix view label command localized * chore: save * Fixes microsoft#157015 * Fixes microsoft#157015 * Fixes microsoft#157015 * Fixes microsoft#157015 * Fixes microsoft#157015 * Fixes microsoft#157015 * fix: use nls.localize2 to simplified code * fix: use nls.localize2 to simplified code * fix: use nls.localize2 to simplified code --------- Co-authored-by: Johannes Rieken <[email protected]> Co-authored-by: Isidor Nikolic <[email protected]> Co-authored-by: Megan Rogge <[email protected]> Co-authored-by: Ulugbek Abdullaev <[email protected]>

…tring` (microsoft#195709) Fixes microsoft#195518 regressed in microsoft#193544

fix: to microsoft#157015 fix view label command localized

9e101ca

vscodenpa added the triage-needed label Sep 20, 2023

vscodenpa assigned alexr00 Sep 20, 2023

alexr00 assigned TylerLeonhardt and unassigned alexr00 Sep 20, 2023

TylerLeonhardt added this to the Backlog milestone Sep 21, 2023

yiliang114 added 3 commits September 22, 2023 10:33

Merge branch 'main' into zonal-antlion

2b9fb45

chore: save

52148ae

Fixes microsoft#157015

c7515e2

yiliang114 commented Sep 22, 2023

View reviewed changes

src/vs/workbench/contrib/quickaccess/browser/viewQuickAccess.ts Outdated Show resolved Hide resolved

TylerLeonhardt reviewed Sep 25, 2023

View reviewed changes

src/vs/workbench/contrib/editSessions/browser/editSessionsViews.ts Outdated Show resolved Hide resolved

TylerLeonhardt reviewed Sep 25, 2023

View reviewed changes

src/vs/workbench/contrib/userDataSync/browser/userDataSyncViews.ts Outdated Show resolved Hide resolved

TylerLeonhardt reviewed Sep 25, 2023

View reviewed changes

src/vs/workbench/services/userDataProfile/browser/userDataProfileImportExportService.ts Show resolved Hide resolved

TylerLeonhardt reviewed Sep 25, 2023

View reviewed changes

jrieken and others added 8 commits September 25, 2023 11:20

Merge pull request microsoft#192602 from sandersn/ai-codefixes

98b88b3

Copilot-based TS refactors

Merge pull request microsoft#191116 from microsoft/isidorn/thick-rein…

f98ef49

…deer add workspace tags

Merge pull request microsoft#193720 from microsoft/merogge/diff

19e2249

add comment help info to diff help dialog

Merge branch 'main' into ulugbekna/implement-code-mappers-context

348b842

Merge pull request microsoft#193656 from microsoft/ulugbekna/implemen…

4718459

…t-code-mappers-context support getting various document ranges from InteractiveSessionProviders

Merge branch 'main' into zonal-antlion

1c2b2a3

Fixes microsoft#157015

f1b0fbf

Fixes microsoft#157015

896e2ea

yiliang114 force-pushed the zonal-antlion branch from 0328d1f to 896e2ea Compare September 25, 2023 04:37

TylerLeonhardt reviewed Sep 25, 2023

View reviewed changes

src/vs/workbench/browser/parts/views/viewsService.ts Outdated Show resolved Hide resolved

yiliang114 requested a review from TylerLeonhardt September 26, 2023 08:12

TylerLeonhardt modified the milestones: Backlog, October 2023 Sep 27, 2023

TylerLeonhardt reviewed Sep 29, 2023

View reviewed changes

src/vs/workbench/services/views/test/browser/viewDescriptorService.test.ts Show resolved Hide resolved

yiliang114 force-pushed the zonal-antlion branch from 2687aa3 to c91a12f Compare September 30, 2023 05:04

TylerLeonhardt removed the triage-needed label Oct 4, 2023

ulugbekna and others added 7 commits October 9, 2023 10:45

Merge branch 'main' into ulugbekna/implement-code-mappers-context

74bf93d

Merge pull request microsoft#193656 from microsoft/ulugbekna/implemen…

4a92285

…t-code-mappers-context support getting various document ranges from InteractiveSessionProviders

Merge branch 'main' into zonal-antlion

1c8e12a

Fixes microsoft#157015

5bd253e

Fixes microsoft#157015

c89cb5a

Fixes microsoft#157015

3a91bfa

fix: use nls.localize2 to simplified code

5fd1e02

yiliang114 force-pushed the zonal-antlion branch from c91a12f to 5fd1e02 Compare October 9, 2023 02:46

yiliang114 added 2 commits October 9, 2023 11:05

fix: use nls.localize2 to simplified code

02d0d71

fix: use nls.localize2 to simplified code

06f2648

yiliang114 requested a review from TylerLeonhardt October 9, 2023 03:21

Merge branch 'main' into zonal-antlion

9bcc960

TylerLeonhardt approved these changes Oct 9, 2023

View reviewed changes

TylerLeonhardt enabled auto-merge (squash) October 9, 2023 22:34

roblourens reviewed Oct 10, 2023

View reviewed changes

roblourens approved these changes Oct 10, 2023

View reviewed changes

TylerLeonhardt merged commit eb4f3c1 into microsoft:main Oct 10, 2023

TylerLeonhardt added a commit that referenced this pull request Oct 16, 2023

Use .value since the type had changed from string to `ILocalizedS…

45f69db

…tring` Fixes #195518 regressed in #193544

TylerLeonhardt mentioned this pull request Oct 16, 2023

Use .value since the type had changed from string to ILocalizedString #195709

Merged

TylerLeonhardt added a commit that referenced this pull request Oct 16, 2023

Use .value since the type had changed from string to `ILocalizedS…

3cdcf0a

…tring` (#195709) Fixes #195518 regressed in #193544

Alex0007 pushed a commit to Alex0007/vscode that referenced this pull request Oct 26, 2023

Use .value since the type had changed from string to `ILocalizedS…

a2ff345

…tring` (microsoft#195709) Fixes microsoft#195518 regressed in microsoft#193544

github-actions bot locked and limited conversation to collaborators Nov 24, 2023

Conversation

yiliang114 commented Sep 20, 2023 • edited by TylerLeonhardt Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TylerLeonhardt commented Sep 21, 2023

Uh oh!

Uh oh!

yiliang114 commented Sep 22, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

TylerLeonhardt left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

TylerLeonhardt commented Sep 27, 2023

Uh oh!

Uh oh!

TylerLeonhardt commented Oct 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yiliang114 commented Oct 7, 2023

Uh oh!

yiliang114 commented Oct 9, 2023

Uh oh!

TylerLeonhardt left a comment

Choose a reason for hiding this comment

Uh oh!

roblourens Oct 10, 2023

Choose a reason for hiding this comment

Uh oh!

TylerLeonhardt Oct 10, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

yiliang114 commented Sep 20, 2023 •

edited by TylerLeonhardt

Loading

TylerLeonhardt commented Oct 4, 2023 •

edited

Loading