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

Extra newline is inserted after beautifying code with anonymous function #1085

Closed
alundiak opened this issue Dec 27, 2016 · 6 comments
Closed
Milestone

Comments

@alundiak
Copy link

alundiak commented Dec 27, 2016

Description

Beautifying introduce extra new line in code. But I'm not sure it's bug.

Input

The code looked like this before beautification:

this.model.myMethod().then(function() {
	// code
}).catch(
	function() {
		// code
	}
);

1 empty line after catch( and before function() {

Expected Output

Now I am not sure what is expected.

Actual Output

The code actually looked like this after beautification:

this.model.myMethod().then(function() {
	// code
}).catch(

	function() {
		// code
	}
);

FYI

Such code is not changed by Beautifying:

this.model.myMethod().then(function() {
    // code
}).catch(function() {
    // code
});

Steps to Reproduce

  • Install usage of grunt-jqbeautifier v 0.2.7 (it has dependency written in form "js-beautify": ">=1.4.2",)
  • Verifiy that jsbeautifer installed 1.6.6

Environment

OS: Windows.
NodeJS 4.4.0, npm 2.4.12, grunt v 0.4.5

Settings

Example:
default by jsbeautify jsbeautifyrc

{
    "indent_size": 4,
    "indent_char": " ",
    "indent_level": 0,
    "indent_with_tabs": false,
    "preserve_newlines": true,
    "max_preserve_newlines": 10,
    "jslint_happy": false,
    "space_after_anon_function": false,
    "brace_style": "collapse,preserve-inline",
    "keep_array_indentation": false,
    "keep_function_indentation": false,
    "space_before_conditional": true,
    "break_chained_methods": false,
    "eval_code": false,
    "unescape_strings": false,
    "wrap_line_length": 0
}

My codebase grunt task:

jsbeautifier: {
	files: [srcDir + '/**/*.js', '!' + bowerDir + '/**', testDir + '/**/*.js'],
	options: {
		js: {
			indentWithTabs: true
		}
	}
},

Not sure, but maybe it's somehow related to the same cause as in #1060 or described.

@alundiak alundiak changed the title Extra newline is inserted after beautifying code with catch and anonymous function Extra newline is inserted after beautifying code with anonymous function Dec 27, 2016
@wcarmon
Copy link

wcarmon commented Dec 28, 2016

+1 (we are experiencing the same with 1.6.6. Works fine in 1.6.4)

@bitwiseman
Copy link
Member

@alundiak -
Thank you for the high-quality bug report.

function declarations that are not on the same line as the parenthesis are suppose to have a blank line before them.

For example, In a statement block:

a();

function a() {
    // Do stuff
}

function b() {
    // Do stuff
}

function c() {
    // Do stuff
}

I can see how, as a parameter of a method call, this might not be ideal.

@wcarmon - I see the same behavior between 1.6.6 and 1.6.4 for the given input. Please clarify.

@gagern
Copy link

gagern commented Dec 29, 2016

Another use case where I'd rather avoid empty lines is when writing fallback functions like this:

var transformer =
    options.transformer ||
    globalSettings.transformer ||
    function(x) {
        return x;
    };

I'd say a function should have a preceding empty line if it is either declared in a statement, or used in an expression which is of the simple form ‹LHS› = function(…) {…}. Everything else should have newlines as given in the original code. This should cover uses of a function as an argument, or inside expressions like my || chain.

By the way, I too see this issue here and #1090 in our actual code base with 1.6.7 while 1.6.2 has no such issues. I've not yet worked out what aspect of the surrounding code makes this a new problem.

@gagern
Copy link

gagern commented Dec 29, 2016

I did a quick git bisect. Both the original example and my test case got broken by 5fb9542 for 1.6.5.

@alundiak
Copy link
Author

alundiak commented Dec 29, 2016

@bitwiseman Can we assume/treat mentioned SHA by @gagern as a bug in jsbeautify and it will be fixed in nearest release? Or this described behavior by me is closer to standard behavior of new jsBeautifier?

PS: How I spotted on this. I had setup of node modules, including grunt-jsbeautifier, and based on version setting of jsbeautify as dependency I receive new release when I did my project clean setup. Image my surprise, when a few files were beautified in a bit different way. So now I decide what the standard is/should be.

@bitwiseman bitwiseman added this to the v1.7.x milestone Dec 30, 2016
@bitwiseman
Copy link
Member

@gagern - Again, good example and well said.
@alundiak - between your and gagern's examples, this case is definitely a bug, but I want to make sure we don't break related cases while fixing this.

Just to clarify, you'd like this to be unchanged:

this.model.myMethod().then(function() {
	// code
}).catch(
	function() {
		// code
	},
	function() {
		// code
	}
);

@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

4 participants