-
Notifications
You must be signed in to change notification settings - Fork 57
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
Fixing the issue of striping attributes when beautify #458
base: main
Are you sure you want to change the base?
Fixing the issue of striping attributes when beautify #458
Conversation
…-online-tools" This reverts commit 78ec797, reversing changes made to 9b1f727.
@jlovejoy Can you review this? |
@rtgdk - I'm not sure how the functionality for the "beautify" option works or if this is all that is needed to fix this issue. Can you please review?? |
src/app/formatxml.py
Outdated
@@ -13,6 +13,7 @@ | |||
|
|||
INDENT = 2 | |||
LINE_LENGTH = 80 | |||
IS_EXCEPTION = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having a global variable can affect other users as well since the global value is changed. The application won't have separate instances of app
for every user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments
src/app/formatxml.py
Outdated
@@ -143,7 +152,9 @@ def pretty(node, level): | |||
text = text.replace('&', '&').replace('>', '>').replace('<', '<') | |||
ser += text | |||
for child in node: | |||
ser += pretty(child, child_level) | |||
res = pretty(child, child_level) | |||
if not(IS_EXCEPTION and 'standardLicenseHeader' in res): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs to checked in the context of pretty
func not at a global level I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -143,7 +148,9 @@ def pretty(node, level): | |||
text = text.replace('&', '&').replace('>', '>').replace('<', '<') | |||
ser += text | |||
for child in node: | |||
ser += pretty(child, child_level) | |||
res = pretty(child, child_level) | |||
if not('exception' in ser and 'standardLicenseHeader' in res): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
E275: missing whitespace after keyword
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
Command | Usage |
---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Help us improve LIFT! (Sonatype LiftBot external survey)
Was this a good recommendation for you? Answering this survey will not impact your Lift settings.
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sonatype-lift ignore
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've recorded this as ignored for this pull request.
If you change your mind, just comment @sonatype-lift unignore
.
@BanulaKumarage @rtgdk - Should this be merged? |
formatlXml.py
by adding exception with its allowed attributes.standardLicenseHeader
tag is not allowed. Therefore it is removed when submitting an exception.