Skip to content

XML Import/Export commands shouldn't assume a server connection in a multi-root workspace #1387

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 1 commit into from
Jun 26, 2024

Conversation

isc-bsaviano
Copy link
Contributor

This PR addresses some confusing behavior reported by an internal dev. They had a multi-root workspace connected to %SYS and USER on the same server. They had a document from %SYS in the active editor tab, but wanted to import an XML file into USER. The command defaulted to use the %SYS connection, so the file got imported into the wrong namespace. I think it's a better user experience to always ask for the connection if you're in a multi-root workspace.

Copy link
Contributor

@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.

Does this change have any impact on people who work with the client-side editing paradigm?

@isc-bsaviano
Copy link
Contributor Author

No, the behavior is the same for client-side and server-side workspaces. The only change is that the command won't use the server connection of the active text editor by default.

Copy link
Contributor

@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.

Approved on the basis of code-reading only.

@isc-bsaviano isc-bsaviano merged commit 5cfc9ec into intersystems-community:master Jun 26, 2024
5 checks passed
@isc-bsaviano isc-bsaviano deleted the xml branch June 26, 2024 13:41
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.

2 participants