Skip to content
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

Edit Settings command opens JSON file #230

Merged
merged 3 commits into from
Jul 10, 2024

Conversation

isc-bsaviano
Copy link
Contributor

This PR fixes #227

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 also been looking at this, and in future we should probably do a better job of coordinating who's working on what.

Opening settings as JSON at a particular key can already be done more simply:

	vscode.commands.executeCommand("workbench.action.openSettingsJson", { revealSetting: { key: "intersystems.servers" } });

But because of the way our package.json defines this object the following doesn't go to the specific entry:

	vscode.commands.executeCommand("workbench.action.openSettingsJson", { revealSetting: { key: `intersystems.servers.${server.name}` } });

So I suggest you refine your revealServers technique to find the sub-object that matches server.name (perhaps rename the function revealServer).

It might be safer not to select any characters though, as doing so creates a risk of the user inadvertently deleting or overtyping.

@isc-bsaviano
Copy link
Contributor Author

Sorry John, I should have assigned myself. I didn't know about revealSetting, I will definitely adopt that. I see your point about the selection potentially being dangerous. I'll remove that as well.

@gjsjohnmurray
Copy link
Collaborator

revealSetting should be able to get you close, but I suggest you augment it with a forward scan to find the definition of the actual server the user invoked the command from.

src/extension.ts Outdated Show resolved Hide resolved
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.

👍

@isc-bsaviano isc-bsaviano merged commit 150d824 into intersystems-community:master Jul 10, 2024
5 checks passed
@isc-bsaviano isc-bsaviano deleted the fix-227 branch July 10, 2024 13:34
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.

Easy Access to setting.json
2 participants