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

Capture empty tags #1868

Merged
merged 4 commits into from
Dec 23, 2020
Merged

Capture empty tags #1868

merged 4 commits into from
Dec 23, 2020

Conversation

JuanFML
Copy link
Contributor

@JuanFML JuanFML commented Dec 23, 2020

Description

Updated xml to capture empty tags in both python and JS files

  • Source branch in your fork has meaningful name (not master)

Fixes Issue: #1854

Before Merge Checklist

These items can be completed after PR is created.

(Check any items that are not applicable (NA) for this PR)

  • JavaScript implementation
  • Python implementation (NA if HTML beautifier)
  • Added Tests to data file(s)
  • Added command-line option(s) (NA if
  • README.md documents new feature/option(s)

Added validations to prevent tagged literals from being modified
Copy link
Member

@bitwiseman bitwiseman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice change.

Comment on lines 2499 to 2502
' <a>',
' <td>Hello</td>',
' <td>World</td>',
' </a>',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be empty to actually test your change right?

Suggested change
' <a>',
' <td>Hello</td>',
' <td>World</td>',
' </a>',
' <>',
' <td>Hello</td>',
' <td>World</td>',
' </>',

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! I didn't see I hadn't changed it back, I will change this now

@@ -126,7 +126,7 @@ var Tokenizer = function(input_string, options) {
html_comment_end: pattern_reader.matching(/-->/),
include: pattern_reader.starting_with(/#include/).until_after(acorn.lineBreak),
shebang: pattern_reader.starting_with(/#!/).until_after(acorn.lineBreak),
xml: pattern_reader.matching(/[\s\S]*?<(\/?)([-a-zA-Z:0-9_.]+|{[\s\S]+?}|!\[CDATA\[[\s\S]*?\]\])(\s+{[\s\S]+?}|\s+[-a-zA-Z:0-9_.]+|\s+[-a-zA-Z:0-9_.]+\s*=\s*('[^']*'|"[^"]*"|{[\s\S]+?}))*\s*(\/?)\s*>/),
xml: pattern_reader.matching(/[\s\S]*?<(\/?)([-a-zA-Z:0-9_.]+|{[\s\S]+?}|!\[CDATA\[[\s\S]*?\]\]|)(\s+{[\s\S]+?}|\s+[-a-zA-Z:0-9_.]+|\s+[-a-zA-Z:0-9_.]+\s*=\s*('[^']*'|"[^"]*"|{[\s\S]+?}))*\s*(\/?)\s*>/),
Copy link
Member

@bitwiseman bitwiseman Dec 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is clearer to say this group is optional, but that might cause the number groups to change, right?

This is not a suggestion, I'm just using that to show a clear diff.

Suggested change
xml: pattern_reader.matching(/[\s\S]*?<(\/?)([-a-zA-Z:0-9_.]+|{[\s\S]+?}|!\[CDATA\[[\s\S]*?\]\]|)(\s+{[\s\S]+?}|\s+[-a-zA-Z:0-9_.]+|\s+[-a-zA-Z:0-9_.]+\s*=\s*('[^']*'|"[^"]*"|{[\s\S]+?}))*\s*(\/?)\s*>/),
xml: pattern_reader.matching(/[\s\S]*?<(\/?)([-a-zA-Z:0-9_.]+|{[\s\S]+?}|!\[CDATA\[[\s\S]*?\]\])?(\s+{[\s\S]+?}|\s+[-a-zA-Z:0-9_.]+|\s+[-a-zA-Z:0-9_.]+\s*=\s*('[^']*'|"[^"]*"|{[\s\S]+?}))*\s*(\/?)\s*>/),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see what you mean, but yes it causes other problems

@JuanFML
Copy link
Contributor Author

JuanFML commented Dec 23, 2020

While fixing this issue I saw something that could be another issue:
In https://beautifier.io/ whenever you try to beautify a code with an indent of 2 spaces to an indent with 4 spaces it is unable to beautify it correctly:
Input: (with predefined options only with Support e4x/jsx syntax selected)

class Columns extends React.Component {
  render() {
    return (
      <t>        
        <td>Hello</td>
        <td>World</td>
      </t>
    );
  }
}

Output:

class Columns extends React.Component {
    render() {
        return (
            <t>        
        <td>Hello</td>
        <td>World</td>
      </t>
        );
    }
}

Although if I changed to indent to 2 spaces per indent, the output would be unchanged from the first input which is the expected output. This is because the input is tabbed with 2 spaces. Maybe the error is because of the single tags, although I haven't looked into it yet

@bitwiseman
Copy link
Member

@JuanFML
Yes, you're seeing #665.

@bitwiseman bitwiseman merged commit 4c25f59 into beautifier:master Dec 23, 2020
@JuanFML
Copy link
Contributor Author

JuanFML commented Dec 23, 2020

@JuanFML
Yes, you're seeing #665.

Oh yes that it is what I encounter, I see now it was already reported

@Zetrick
Copy link

Zetrick commented Mar 3, 2022

I am still having the issue of empty <> </> tags in my jsx being incorrectly formatted. Do I need to change something manually, or add another setting to the .jsbeautifyrc file to incorporate this change?

Before format:

<>
    <ExampleComponentA />
    <ExampleComponentB />
</>

After format:

<>
    <ExampleComponentA /> <
ExampleComponentB / >
<
/>

@bitwiseman
Copy link
Member

bitwiseman commented Apr 5, 2022

@Zetrick
Not sure. have your tried this on https://beautifier.io ? Looks good to me there.

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.

3 participants