royriojas / esformatter-jsx

esformatter plugin: format jsx files (or js files with Facebook React JSX Syntax)
MIT License
142 stars 25 forks source link

esformatter doesn't preserve newlines #75

Closed fab1an closed 6 years ago

fab1an commented 8 years ago

For some reason esformatter won't preserve any newlines. My config:

    "jsx": {
        "attrsOnSameLineAsTag": false,
        "maxAttrsOnTag": 1,
        "firstAttributeOnSameLine": false,
        "alignWithFirstAttribute": false,
        "spaceInJSXExpressionContainers": "",
        "JSXExpressionsSingleLine": false,
        "htmlOptions": {
            "indent_size": 4,
            "max_preserve_newlines": 10,
            "preserve_newlines": true
        }
    },
fab1an commented 8 years ago

Is there anything I can do about this? The problem is that it reformats my Text and span attributes which changes the way text is displayed, so formatting actually introduces bugs :(

royriojas commented 8 years ago

hi @fab1an, sorry for the issues on preserving new lines

can you please send me the following:

  1. A sample of code previous to formatting
  2. A sample of formatted code with the issue
  3. A sample of the desired output
fab1an commented 8 years ago

Prior:

class Test extends React.Component {
    render() {
        return (
            <div>
                       <div />

             <div />
   </div>
        )
    }
}

After

class Test extends React.Component {
    render() {
        return (
            <div>
                <div />
                <div />
            </div>
        )
    }
}

Desired output:

class Test extends React.Component {
    render() {
        return (
            <div>
                <div />
                /* newline! */
                <div />
            </div>
        )
    }
}

The newline between the <div/> disappeared.

Config:

{
    "plugins": [
        "esformatter-jsx"
    ],
    "jsx": {
        "attrsOnSameLineAsTag": false,
        "maxAttrsOnTag": 1,
        "firstAttributeOnSameLine": false,
        "alignWithFirstAttribute": false,
        "spaceInJSXExpressionContainers": "",
        "JSXExpressionsSingleLine": false,
        "htmlOptions": {
            "indent_char": " ",
            "indent_size": 4,
            "preserve_newlines": true
        }
    },
    "indent": {
        "value": "    "
    }
}
fab1an commented 8 years ago

And thanks for your time!

fab1an commented 8 years ago

Any news on this?

royriojas commented 8 years ago

hi @fab1an, I'm sorry I haven't had the time yet to take a look to it.

It seems simple though, if someone wants to help with this PRs are welcome!

Regards

fab1an commented 8 years ago

I don't have time for more than spotting simple bugs either :-/.

I've looked through the code, and from what I see esformatter-jsx send the html "between" the jsx separately through js-beautify. If I'm right this is the reason the options regarding newlines don't work, because there are none.

jamtat commented 7 years ago

This is a dupe of https://github.com/royriojas/esformatter-jsx/issues/22

jamtat commented 7 years ago

Part of the problem is https://github.com/royriojas/esformatter-jsx/blob/master/src/format-jsx.js#L155 ( return removeEmptyLines(code.toString()); ) which calls removeEmptyLines on the source regardless of the preserve_newlines option. It should probably not do that.

Likewise https://github.com/royriojas/esformatter-jsx/blob/master/src/format-jsx.js#L51 for the same reason.

royriojas commented 7 years ago

Hi @jamtat you're right. The code needs some refactor. Really hate that I'm so busy these days, that I can't really make any progress on these issues. :( That said. PR's are most than welcome.

jamtat commented 7 years ago

I made a fork earlier and pushed a commit to conditionallly remove blank lines but it has a bug that inserts whitespace around JSXExpressions (I think?) Which means most of the tests fail. I'll keep having little attempts at fixing it and PR when the tests pass :)

On Mon, 10 Apr 2017 at 19:03, Roy Riojas notifications@github.com wrote:

Hi @jamtat https://github.com/jamtat you're right. The code needs some refactor. Really hate that I'm so busy these days, that I can't really make any progress on these issues. :( That said. PR's are most than welcome.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/royriojas/esformatter-jsx/issues/75#issuecomment-293030393, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOtg2t7lZAuK3snc4NOBp_pumO70yRkks5rum8FgaJpZM4ItZJv .