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

obsolete.py script improvements #364

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

vanepp
Copy link
Contributor

@vanepp vanepp commented Mar 6, 2023

An replacement of the original pull request described in issue #347 Added writing the path to bins/core.fzb and made the changes in the repository (rather than copying a file across) so the change now diffs in git

@vanepp
Copy link
Contributor Author

vanepp commented Mar 9, 2023

I just came across a potential issue with (probably both) obsolete.py implementations. Minidom appears sometime reformat the xml on output as in this case in core.fzb

diff --git a/bins/core.fzb b/bins/core.fzb
index 39706e9..145199f 100644
--- a/bins/core.fzb
+++ b/bins/core.fzb
@@ -1,11 +1,10 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<module fritzingVersion="0.4.3b.0.0" icon="Core.png">
+<?xml version="1.0" encoding="UTF-8" standalone="no"?><module fritzingVersion="0.4.3b.0.0" icon="Core.png">
        <title>Core Parts</title>
     <instances>
         <instance moduleIdRef="__spacer__" modelIndex="3" path="Basic">
             <views>
               <iconView layer="icon">
-                <geometry z="-1" x="-1" y="-1"></geometry>
+                <geometry z="-1" x="-1" y="-1"/>
               </iconView>
             </views>
         </instance>

The first line shouldn't be a problem, it just added the "standalone="no"?" that I told it to. I don't think the next one where

</geometry>

gets replaced by

/>

should be a problem either because they are identical xml (I think at least!) but I thought I should point it out here in case there is an issue I'm not aware of. In this case, this should only occur on the first instance (which will be this one when I get the pull request together!) in to core parts, after that the core.fzb file will be in the format the Minidom outputs. I expect similar issues may show up in other xml as it is obsoleted by obsolete,py but I'm not sure how we could fix it easily as it doesn't appear to be prettyprinting the xml which was my first thought. I don't see any option to pass the xml through unchanged, but I'm not all the familiar with Minidom either. I think I may be able to convince lxml to pass the xml through unmodified (although I'm not sure of that either!) but that would require loading lxml in to python which is probably less desirable than using Minidom which is built in.

edit:

a bit further down I found something that may be a problem:

                 </iconView>
             </views>
         </instance>
-        <instance moduleIdRef="blueIOT_Rc1-final" modelIndex="1" path="blueIOT_Rc1-final.fzp">>
+        <instance moduleIdRef="blueIOT_Rc1-final" modelIndex="1" path="blueIOT_Rc1-final.fzp">&gt;
           <views>
               <iconView layer="icon">
                   <geometry z="-1" x="-1" y="-1"/>
               </iconView>
           </views>
         </instance>

I think this will add an extra &gt which shouldn't be there (although it is there now with no apparent ill effects.) It may be that the extra greater than just gets ignored as stray text? It looks to be a mistake in editing the file at some point that never got caught.

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.

1 participant