Skip to content
This repository was archived by the owner on Mar 26, 2026. It is now read-only.

Restructure extension code and add fixes for Windows paths and views not loading#83

Merged
worksofliam merged 11 commits intomainfrom
refactor/source-orbit-tree-item
Oct 8, 2024
Merged

Restructure extension code and add fixes for Windows paths and views not loading#83
worksofliam merged 11 commits intomainfrom
refactor/source-orbit-tree-item

Conversation

@SanjulaGanepola
Copy link
Copy Markdown
Member

@SanjulaGanepola SanjulaGanepola commented Oct 4, 2024

Changes

  • Major restructuring of files
  • Update extension details in package.json
  • Removed vscode-sourceorbit:projectsLoaded context in favor of having views always shown with welcome view text
  • Fix windows path issue in getImpacts request in server code
  • Fix views not rendering anything if project not loaded
  • Fix affected object view not showing anything when extension first loaded and file is open

I have more changes to make, but will add them in separate smaller PRs


Note added by @worksofliam: this only affects vs/

Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
@worksofliam
Copy link
Copy Markdown
Member

@SanjulaGanepola I can get this review started on Monday. Fab work!

@SanjulaGanepola SanjulaGanepola added enhancement New feature or request vs Specific to the Visual Studio Extension labels Oct 4, 2024
@edmundreinhardt
Copy link
Copy Markdown
Member

@SanjulaGanepola I can get this review started on Monday. Fab work!

I totally agree that this is fantastic!
I will try to get to it by Monday, I will be in Rochester for LUG

Comment thread vs/client/src/views/projectExplorer/ileObjectTreeItem.ts
@worksofliam
Copy link
Copy Markdown
Member

@SanjulaGanepola which repos are you testing this with? ibmi-company_system and bob-recursive-example?

@worksofliam
Copy link
Copy Markdown
Member

@SanjulaGanepola This view under source control is always showing the welcome page. This is for company_system. Any clues?

image

@SanjulaGanepola
Copy link
Copy Markdown
Member Author

@worksofliam That is odd cuz it appears fine for me. Does the active impact view work? The only real change I made to this logic was https://github.com/IBM/sourceorbit/pull/83/files#diff-5a23ea1f11048195dc7ad0fdf0d21e5b66e20339b68c5f1a7e42a1ccef0fe085R28

image

@worksofliam
Copy link
Copy Markdown
Member

worksofliam commented Oct 7, 2024

@SanjulaGanepola Yup, it works! But, I think we need to give the two 'Affected Objects' windows different names (which is totally my bad, because I think I wrote that 😂)

Perhaps the 'Affected Objects' under Explorer should be 'Source Effects' and the one under Source Control should be 'Affected Objects' (no change).

What do you think?

@edmundreinhardt
Copy link
Copy Markdown
Member

@SanjulaGanepola Yup, it works! But, I think we need to give the two 'Affected Objects' windows need to have different names (which is totally my bad, because I think I wrote that 😂)

Perhaps the 'Affected Objects' under Explorer should be 'Source Effects' and the one under Source Control should be 'Affected Objects' (no change).

What do you think?

"Source Impacts" for source in editor
"Change Impacts" source in commit

@SanjulaGanepola
Copy link
Copy Markdown
Member Author

@worksofliam Oh wait, were you testing with the company_system test fixture. I was using the actual https://github.com/IBM/ibmi-company_system project. Since we have that in our .gitignore, I think it makes sense we don't show any Git related impacts.

I did remove it from our .gitignore to test it out and did find another bug though with this logic not detecting the repo. I assume we want to support opening any folder within the project being opened as a workspace folder. I will push a fix for this.

we need to give the two 'Affected Objects' windows different names

Totally agree. I think Edmund's suggestion makes sense for the names. I will push this change too.

@SanjulaGanepola
Copy link
Copy Markdown
Member Author

Nevermind...you are using ibmi-company-system. Also forgot your source control view literally has detected changes 😅. Still investigating...

@SanjulaGanepola
Copy link
Copy Markdown
Member Author

I did remove it from our .gitignore to test it out and did find another bug though with this logic not detecting the repo. I assume we want to support opening any folder within the project being opened as a workspace folder. I will push a fix for this.

@worksofliam Ok so that part can be fixed by changing:

const repository = gitApi.repositories.find(r => r.rootUri.fsPath === workspaceFolder.uri.fsPath);

to:

const repository = gitApi.getRepository(workspaceFolder.uri);

but problems could arise here, if there are changes in the project, but they are outside the current workspace folder as a sub folder is opened. Do we need to support opening sub folders?

@worksofliam
Copy link
Copy Markdown
Member

@SanjulaGanepola In theory no sub folders, just multiple workspace uris.

Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
@SanjulaGanepola
Copy link
Copy Markdown
Member Author

This view under source control is always showing the welcome page.

@worksofliam I can't quite tell what broke this since I can't reproduce it. Could you debug this spot and share any findings?

@worksofliam
Copy link
Copy Markdown
Member

worksofliam commented Oct 7, 2024

@SanjulaGanepola for the record, I don't think anything is wrong with the code. I think there was one instance when the git extension did not trigger the change event and so our view didn't update. If I manually refresh, or change a file, the view updates.

edit: i can confirm it works as expected. Likely my bad. Thanks for renaming the columns

@worksofliam
Copy link
Copy Markdown
Member

@SanjulaGanepola Simple suggestion: you have a file named impactView.ts inside of the impactView folder. Might I recommend renaming impactView.ts to index.ts so the imports become simpler?

@worksofliam
Copy link
Copy Markdown
Member

worksofliam commented Oct 7, 2024

@SanjulaGanepola I've got this lint rule being throw in api.ts:

image

edit: same with GitEventHandler in gitEventHandler.ts

Comment thread vs/client/src/languageClientManager.ts Outdated
Comment thread vs/client/src/tasks.ts
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
Signed-off-by: Sanjula Ganepola <Sanjula.Ganepola@ibm.com>
@SanjulaGanepola
Copy link
Copy Markdown
Member Author

SanjulaGanepola commented Oct 7, 2024

@worksofliam Addressed the comments. I was in favor of disabling that specific lint rule so we can use namespaces. Lmk if you think we should revert this.

@worksofliam
Copy link
Copy Markdown
Member

@SanjulaGanepola i am okay with disabled that rule, though only because we don't really have an exportable API. If we do in the future, we should enable it again.

I will review one more time tomorrow and likely merge it now. Then I'll start the next PR - thanks for your work here!

@worksofliam worksofliam merged commit 7290ec2 into main Oct 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request vs Specific to the Visual Studio Extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants