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

Replace XML formatter and add format shortcut #143

Merged
merged 4 commits into from
Aug 22, 2022
Merged

Replace XML formatter and add format shortcut #143

merged 4 commits into from
Aug 22, 2022

Conversation

sonnyp
Copy link
Contributor

@sonnyp sonnyp commented Aug 17, 2022

The shortcut is <Ctrl><Shift>Return.

There were a few issues with our previous XML formatter, the biggest one is that it would behave differently than Blueprint formatter. So I wrote a custom XML formatter that behaves exactly like Blueprint xml_emitter.py .

https://gitlab.gnome.org/jwestman/blueprint-compiler/-/blob/59283a76adc8d270ff5f67b630b7dfa905ec34a9/blueprintcompiler/xml_emitter.py

@melix99 could you test this quickly? Building this branch in GNOME Builder should work just fine.

Feel free to review as well but that's not required .

Fixes #141

There were a few issues with our previous XML formatter, the biggest one
is that it would behave differently than Blueprint formatter. So I wrote
a custom XML formatter that behaves exactly like Blueprint
xml_emitter.py .

https://gitlab.gnome.org/jwestman/blueprint-compiler/-/blob/59283a76adc8d270ff5f67b630b7dfa905ec34a9/blueprintcompiler/xml_emitter.py
src/lib/ltx.js Show resolved Hide resolved
src/window.js Show resolved Hide resolved
@melix99
Copy link

melix99 commented Aug 17, 2022

There's something wrong while formatting XMLs like this:

<?xml version="1.0" encoding="UTF-8"?>
<interface>
  <object class="GtkBox"></object>
</interface>

It deletes code every time I press the shortcut.

@sonnyp
Copy link
Contributor Author

sonnyp commented Aug 17, 2022

@melix99 nice catch — it's fixed

@melix99
Copy link

melix99 commented Aug 18, 2022

Another thing I discovered is that the formatter is a bit too destructive when when the xml is not valid:

<?xml version="1.0" encoding="UTF-8"?>
<interface>
  <object class="GtkBox" hey/>
</interface>

Is formatted to:

<?xml version="1.0" encoding="UTF-8"?>
<interface>

I think it's better to just do nothing or showing an error when the xml is invalid instead of deleting code.

@sonnyp
Copy link
Contributor Author

sonnyp commented Aug 22, 2022

@melix99 fixed as well

Thanks a lot for your help testing this. I will merge this but please do let me know if you encounter an issue or annoyance.

FYI this is a known issue #144

@sonnyp sonnyp merged commit 5f51457 into main Aug 22, 2022
@sonnyp sonnyp deleted the format-shortcut branch August 22, 2022 18:22
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