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

Add serialization prefs when opening, downloading XML documents from database #328

Merged
merged 7 commits into from
Jul 14, 2021

Conversation

joewiz
Copy link
Member

@joewiz joewiz commented Jul 8, 2021

Before this PR eXide hard-coded serialization parameters—indent=yes expand-xincludes=no when opening and downloading XML documents from the database and indent=no expand-xincludes=no when generating and downloading packages—and users had no means to change this behavior. Numerous times the indentation behavior of eXide caused confusion when users opened documents or saved documents and observed whitespace introduced into their documents. See these posts to exist-open:

The only solution was to suggest editing eXide's sources to change the hardcoded default or to use a different editor.

This PR adds preferences for these two serialization parameters to the eXide preferences window:

Screen Shot 2021-07-07 at 5 47 00 PM

The preferences default to the longstanding eXide defaults for opening and downloading individual documents: indent=yes expand-xincludes=no. The preferences also now govern serialization when generating and downloading packages via Application > Download app, and thus the default here is changed from a hardcoded indent=no to follow the eXide Preference.

The load.xql endpoint now supports indent and expand-xincludes parameters, just as deployment.xql had already done.

Alternatives I considered:

  • Showing these preferences in the editor window. I opted to place these options in Preferences since in the main editor window, they would take up valuable space; there's no good place for this in the UI; the "indent on load" checkbox could cause confusion with the "indent" checkbox in the results pane; and we've never had the ability to adjust this before, so it's doubtful we'd want/need to toggle this all of the time. Let's give this a try and see if we really need to toggle this on .
  • Providing distinct preferences for (a) opening/downloading individual documents vs. (b) generating & downloading packages. I opted for a single set of preferences for simplicity's sake; it seems to me that if you like indentation, you'd want it everywhere, but if you don't want indentation, you'll disable it and want to keep it off.
  • Setting the defaults via eXide's configuration.xml file instead of src/preferences.js - but none of the other preferences are set there, and I am not capable of the javascript needed to get the preferences.js file to read eXide's prefs from the configuration.xml file.

Previous work in this area includes:

Deploy command was removed from the Application menu in eXist-db@f3e5c17
Applies to open file operations (via File > Open dialog, the Open toolbar, the directory tab) and download file operations (via File > Download, Application > Download app) when loading XML documents from the database. Default behavior for opening files remains “indent=yes expand-xincludes=no”. Now Application > Download app follows this preference (and defaults) instead of hardcoding “indent=no”. Both load.xql and deployment.xql endpoints respect “indent” and “expand-xincludes” parameters.
@joewiz joewiz requested a review from a team July 14, 2021 14:43
@adamretter adamretter merged commit 7f6273f into eXist-db:develop Jul 14, 2021
@joewiz joewiz deleted the add-prefs-for-serializing-on-open branch July 14, 2021 19:04
@joewiz
Copy link
Member Author

joewiz commented Jul 14, 2021

@adamretter Thank you!

@adamretter
Copy link
Member

@joewiz you are very welcome. I have also assigned @younes to take care of eXist-db/exist#2394 for you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants