Skip to content

Fix Servers tree action buttons for servers defined in a workspace folder #287

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 2 commits into from
Jul 28, 2025

Conversation

isc-bsaviano
Copy link
Collaborator

  • Fixes No actions possible #284
  • Hide the view and edit code commands. It doesn't make sense to allow the creation of server-side workspace folders for servers that are defined within a workspace folder. The created server-side folders will not work.
  • Fix the SMP commands.

@gjsjohnmurray
Copy link
Collaborator

I think the addition of a suffix risks affecting other extensions that may be adding commands on tree items whose context keys end with "namespace". Instead of breaking those, could your solution add a prefix which the "add ISFS folder to workspace" commands could test (negatively) for?

Copy link
Collaborator

@gjsjohnmurray gjsjohnmurray left a comment

Choose a reason for hiding this comment

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

I have proposed (untested) changes intended to avoid increasing the number of /-pieces in the context key, as a precaution against breaking any existing extensions. The new wsfolder indicator is now a colon-separated prefix on the existing second slash-separated piece.

@isc-bsaviano
Copy link
Collaborator Author

@gjsjohnmurray I verified that the extra slash portion didn't break anything in the main extension. I think that any change would be backwards-incompatible so unless we have evidence that this specific approach is causing changes I think we should stick with it. What other extensions contribute commands to the Servers tree? Your Webterminal one?

@gjsjohnmurray gjsjohnmurray self-requested a review July 28, 2025 14:14
Copy link
Collaborator

@gjsjohnmurray gjsjohnmurray left a comment

Choose a reason for hiding this comment

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

These are the ones I'm aware of:

image

Based on a quick look at their settings.json files they shouldn't be affected by your change, so I'm approving.

@isc-bsaviano
Copy link
Collaborator Author

Thanks for checking John!

@isc-bsaviano isc-bsaviano merged commit 394a1c0 into intersystems-community:master Jul 28, 2025
3 checks passed
@isc-bsaviano isc-bsaviano deleted the fix-284 branch July 28, 2025 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No actions possible
2 participants