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

XML reformatter sometimes requires multiple tries #19

Closed
klauer opened this issue Mar 2, 2023 · 3 comments · Fixed by #22
Closed

XML reformatter sometimes requires multiple tries #19

klauer opened this issue Mar 2, 2023 · 3 comments · Fixed by #22

Comments

@klauer
Copy link
Contributor

klauer commented Mar 2, 2023

Issue

This hook: https://github.com/pcdshub/pre-commit-hooks/blob/master/pre_commit_hooks/xml_format.py#L1
sometimes requires multiple tries to properly reformat:

  • First try: reformats partially
  • Second try: reformats fully
  • Third try: allows you to commit

We should double-check the code - potentially reformatting with multiple iterations or increasing some parameter to the existing call to dive deeper into the XML.

@ZLLentz
Copy link
Member

ZLLentz commented Jun 21, 2023

Here's of one example of a diff that only includes changes made on the second try, as maybe a clue:

diff --git a/lcls-plc-example-motion/tc_mot_example/tc_mot_example.tmc b/lcls-plc-example-motion/tc_mot_example/tc_mot_example.tmc
index 3ceeaa9..ecd31d7 100644
--- a/lcls-plc-example-motion/tc_mot_example/tc_mot_example.tmc
+++ b/lcls-plc-example-motion/tc_mot_example/tc_mot_example.tmc
@@ -4252,7 +4252,7 @@
       </Method>
       <Method>
         <Name>Write</Name>
-        <Comment>
+        <Comment>
     Writes the contents of the buffer into a file.
 </Comment>
         <ReturnType Namespace="LCLS_General.TcUnit.SysDir.SysTypes">RTS_IEC_RESULT</ReturnType>
@@ -4379,7 +4379,7 @@
       </Method>
       <Method>
         <Name>Find</Name>
-        <Comment>
+        <Comment>
     Find a searchstring in the buffer and returns its position.
     It's possible to add a preffered startposition within buffer
 </Comment>
@@ -4466,7 +4466,7 @@
       </Method>
       <Method>
         <Name>Clear</Name>
-        <Comment>
+        <Comment>
     Clears the buffer and sets the length to 0
 </Comment>
         <Local>
@@ -4919,7 +4919,7 @@
       </Method>
       <Method>
         <Name>ClearBuffer</Name>
-        <Comment>
+        <Comment>
     Clears the contents of the entire buffer.
 </Comment>
       </Method>

I personally can't figure out what's different yet

@ZLLentz
Copy link
Member

ZLLentz commented Jun 21, 2023

After a closer inspection, there is a single space after in these three multiline comment blocks after the first reformat, but not after the second.

@ZLLentz
Copy link
Member

ZLLentz commented Jun 21, 2023

I have two suggestions:

  1. have the parser function run itself on its output a few times
  2. strip whitespace from the end of every line in the output

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 a pull request may close this issue.

2 participants