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

Wrong indent formatting #569

Closed
mbrammer opened this issue Oct 28, 2014 · 9 comments
Closed

Wrong indent formatting #569

mbrammer opened this issue Oct 28, 2014 · 9 comments
Milestone

Comments

@mbrammer
Copy link

Since some js-beautify or other dependency updates the indent is not working correctly anymore.
Example 1:
Before

(function(global) {
    "use strict";

    /* jshint ignore:start */
    include "somefile.js"
    /* jshint ignore:end */
}(this));

After

(function(global) {
    "use strict";

    /* jshint ignore:start */
    include "somefile.js"
        /* jshint ignore:end */
}(this));

Example 2:
Before

(function(global) {
    "use strict";

    $('#level-up').modal({
        backdrop: 'static'
    })
    .find('.text').toggle(textVisibile).end()
    .find('.table').html(games).end()
    .find('.level').html(user.levelUp.newLevel).end()
    .find('.reward').html(user.levelUp.rewards.coins);
}(this));

After

(function(global) {
    "use strict";
    $('#level-up').modal({
            backdrop: 'static'
        })
        .find('.text').toggle(textVisibile).end()
        .find('.table').html(games).end()
        .find('.level').html(user.levelUp.newLevel).end()
        .find('.reward').html(user.levelUp.rewards.coins);
}(this));

Here is my jsbeautifier config:

jsbeautifier: {
    files: ['js/**/*.js'],
    options: {
        js: {
            braceStyle: "collapse",
            breakChainedMethods: false,
            e4x: false,
            evalCode: false,
            indentChar: " ",
            indentLevel: 0,
            indentSize: 4,
            indentWithTabs: false,
            jslintHappy: false,
            keepArrayIndentation: false,
            keepFunctionIndentation: false,
            preserveNewlines: true,
            maxPreserveNewlines: 2,
            spaceBeforeConditional: true,
            spaceInParen: false,
            unescapeStrings: false,
            wrapLineLength: 0
        }
    }
}

And my package.json.

{
  "devDependencies": {
    "grunt": "~0.4.5",
    "grunt-cli": "~0.1.13",
    "grunt-includes": "~0.4.5",
    "grunt-csscomb": "~3.0.0-1",
    "grunt-jsbeautifier": "~0.2.7",
    "grunt-notify": "~0.3.0",
    "grunt-newer": "~0.8.0",
    "grunt-concurrent": "~1.0.0",
    "grunt-contrib-concat": "~0.5.0",
    "grunt-contrib-copy": "~0.7.0",
    "grunt-contrib-uglify": "~0.6.0",
    "grunt-contrib-sass": "~0.8.1",
    "grunt-contrib-compass": "~1.0.1",
    "grunt-contrib-watch": "~0.6.1",
    "grunt-contrib-jshint": "~0.10.0",
    "grunt-contrib-csslint": "~0.3.1",
    "grunt-contrib-imagemin": "~0.9.1",
    "grunt-contrib-nodeunit": "~0.4.1",
    "grunt-lineending": "~0.2.3",
    "jshint-stylish": "~0.2.0",
    "time-grunt": "~1.0.0"
  }
}
@andyburke
Copy link

Also seeing this:

before

    function bindAuthEvent( eventName ) {
        self.auth.on( eventName, function( event, meta ) {
            self.emit( eventName, event, meta );
        } );
    }
    [ 'logged_in', 'logged_out', 'signed_up', 'updated_user' ].forEach( bindAuthEvent );

    function bindBrowserEvent( eventName ) {
        browser.on( eventName, function( event, meta ) {
            self.emit( eventName, event, meta );
        } );
    }
    [ 'navigating' ].forEach( bindBrowserEvent );

after

    function bindAuthEvent( eventName ) {
            self.auth.on( eventName, function( event, meta ) {
                self.emit( eventName, event, meta );
            } );
        }
        [ 'logged_in', 'logged_out', 'signed_up', 'updated_user' ].forEach( bindAuthEvent );

    function bindBrowserEvent( eventName ) {
            browser.on( eventName, function( event, meta ) {
                self.emit( eventName, event, meta );
            } );
        }
        [ 'navigating' ].forEach( bindBrowserEvent );

@andyburke
Copy link

Hello?

With this version, js-beautify is basically doing the exact opposite of what it's supposed to do. At least a comment on this issue would be appreciated.

@andyburke
Copy link

Could it be related to this?: baf2a34

@bitwiseman
Copy link
Member

Hello,
These are very interesting inputs.

Please take each of your formatting errors and file a separate issue for each. I understand you're seeing a bunch of errors, but we'll take them one at a time not as we ball of foo. 😄

@andyburke
Copy link

@bitwiseman all of these formatting errors seem related to the same bug. What's the purpose of filing more bugs when these all are probably driven by a single issue with the indenting logic?

@bitwiseman
Copy link
Member

@andyburke -
The commit you pointed to is specific to indenting inside the conditional expressions for if(...), for(...), and while(...). Very unlikely to be the cause of these issues.

"probably driven by a single issue with the indenting logic"? Have you looked at the code? There's quite a bit of "indenting logic". While it is possible these are the same issue, a cursory look at your examples indicates to me that that they are likely to be caused by different code paths, thus independent issues.

@andyburke
Copy link

I was just trying to be helpful without having to learn your codebase. I looked at recent changes and pointed at what seemed like the most likely one.

As you asked, I have opened a separate issue: #578

I can change the title of that issue to whatever you need it to be. I'm not sure what the case I pointed to is called. Feel free to let me know.

@bitwiseman
Copy link
Member

Cool thanks. 😄
Example 1 ... include is an new keyword? Also, if you put a semicolon at the end of the include line it works. The beautifier has a tough time with semicolonless code. Definitely a bug, but might take some time or be part of a larger enhancement.

Example 2: the same as #482. Technically correct for the current design, possible future enhancement.

@bitwiseman
Copy link
Member

This may be fixed by f9f6566 .

@bitwiseman bitwiseman added this to the v1.7.x milestone Jan 1, 2017
@bitwiseman bitwiseman modified the milestones: v1.7.x, v1.6.8 Jan 2, 2017
0-wiz-0 pushed a commit to NetBSD/pkgsrc-wip that referenced this issue Apr 14, 2017
pkgsrc changes:
 - Use ALTERNATIVES in order to permit multi-pkgs to coexists and adjust
   PLIST and introduce Makefile post-install target accordingly

Changes:
* CSS: Preserve Newlines ([#537](beautifier/js-beautify#537))

Reverted #1117 - Preserve newlines broken

* On beautify, new line before next CSS selector ([#1142](beautifier/js-beautify#1142))

Added `preserver_newlines` to css beautifier

* Fixed html formatting issue with attribute wrap (Thanks, @HookyQR!)
* Fixed python package publishing

* Wrong HTML beautification starting with v1.6.5 ([#1115](beautifier/js-beautify#1115))
* Ignore linebreak when meet handlebar ([#1104](beautifier/js-beautify#1104))
* Lines are not un-indented correctly when attributes are wrapped ([#1103](beautifier/js-beautify#1103))
* force-aligned is not aligned when indenting with tabs ([#1102](beautifier/js-beautify#1102))
* Python package fails to publish  ([#1101](beautifier/js-beautify#1101))
* Explaination of 'operator_position' is absent from README.md ([#1047](beautifier/js-beautify#1047))

* Fixed a batch of comment and semicolon-less code bugs

* Incorrect indentation after loop with comment ([#1090](beautifier/js-beautify#1090))
* Extra newline is inserted after beautifying code with anonymous function ([#1085](beautifier/js-beautify#1085))
* end brace with next comment line make bad indent ([#1043](beautifier/js-beautify#1043))
* Javascript comment in last line doesn't beautify well ([#964](beautifier/js-beautify#964))
* indent doesn't work with comment (jsdoc) ([#913](beautifier/js-beautify#913))
* Wrong indentation, when new line between chained methods ([#892](beautifier/js-beautify#892))
* Comments in a non-semicolon style have extra indent ([#815](beautifier/js-beautify#815))
* [bug] Incorrect indentation due to commented line(s) following a function call with a function argument. ([#713](beautifier/js-beautify#713))
* Wrong indent formatting ([#569](beautifier/js-beautify#569))

Added `content_unformatted` option (Thanks @arai-a)

* HTML pre code indentation ([#928](beautifier/js-beautify#928))
* Beautify script/style tags but ignore their inner JS/CSS content ([#906](beautifier/js-beautify#906))

* Added support for editorconfig from stdin
* Added js-beautify to cdnjs
* Fixed CRLF to LF for HTML and CSS on windows
* Added inheritance/overriding to config format (Thanks @DaniGuardiola and @HookyQR)
* Added `force-align` to `wrap-attributes` (Thanks @LukinoS)
* Added `force-expand-multiline` to `wrap-attributes` (Thanks @tobias-zucali)
* Added `preserve-inline` as independent brace setting (Thanks @Coburn37)
* Fixed handlebars with angle-braces (Thanks @mmsqe)

* Wrong indentation for comment after nested unbraced control constructs ([#1079](beautifier/js-beautify#1079))
* Should prefer breaking the line after operator ? instead of before operator < ([#1073](beautifier/js-beautify#1073))
* New option "force-expand-multiline" for "wrap_attributes" ([#1070](beautifier/js-beautify#1070))
* Breaks if html file starts with comment ([#1068](beautifier/js-beautify#1068))
* collapse-preserve-inline restricts users to collapse brace_style ([#1057](beautifier/js-beautify#1057))
* Parsing failure on numbers with "e" ([#1054](beautifier/js-beautify#1054))
* Issue with Browser Instructions ([#1053](beautifier/js-beautify#1053))
* Add preserve inline function for expand style braces ([#1052](beautifier/js-beautify#1052))
* Update years in LICENSE ([#1038](beautifier/js-beautify#1038))
* JS. Switch with template literals. Unexpected indentation. ([#1030](beautifier/js-beautify#1030))
* The object with spread object formatted not correctly ([#1023](beautifier/js-beautify#1023))
* Bad output generator function in class ([#1013](beautifier/js-beautify#1013))
* Support editorconfig for stdin ([#1012](beautifier/js-beautify#1012))
* Publish to cdnjs ([#992](beautifier/js-beautify#992))
* breaks if handlebars comments contain handlebars tags ([#930](beautifier/js-beautify#930))
* Using jsbeautifyrc is broken ([#929](beautifier/js-beautify#929))
* Option to put HTML attributes on their own lines, aligned ([#916](beautifier/js-beautify#916))
* Erroneously changes CRLF to LF on Windows in HTML and CSS ([#899](beautifier/js-beautify#899))
* Weird space in {get } vs { normal } ([#888](beautifier/js-beautify#888))
* Bad for-of formatting with constant Array ([#875](beautifier/js-beautify#875))
* Problems with filter property in css and scss ([#755](beautifier/js-beautify#755))
* Add "collapse-one-line" option for non-collapse brace styles  ([#487](beautifier/js-beautify#487))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants