Skip to content

Commit

Permalink
Minimal indenting for expressions and arrays
Browse files Browse the repository at this point in the history 
Closes beautifier#281
  • Loading branch information
@bitwiseman
bitwiseman committed Jun 9, 2013
1 parent 812dd55 commit 7c1ff82
Show file tree
Hide file tree
Showing 2 changed files with 109 additions and 46 deletions.
111 changes: 78 additions & 33 deletions js/lib/beautify.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@
var whitespace, wordchar, punct, parser_pos, line_starters, digits;
var prefix;
var input_wanted_newline;
var line_indent_level;
var output_wrapped, output_space_before_token;
var input_length, n_newlines, whitespace_before_token;
var handlers, MODE, opt;
Expand Down Expand Up @@ -134,7 +133,16 @@
};

function create_flags(flags_base, mode) {
var next_indent_level = (flags_base ? flags_base.indentation_level + ((flags_base.var_line && flags_base.var_line_reindented) ? 1 : 0) : 0);
var next_indent_level = 0;
if (flags_base) {
next_indent_level = flags_base.indentation_level;
next_indent_level += (flags_base.var_line && flags_base.var_line_reindented) ? 1 : 0;
if (!just_added_newline() &&
flags_base.line_indent_level > next_indent_level) {
next_indent_level = flags_base.line_indent_level;
}
}

var next_flags = {
mode: mode,
parent: flags_base,
Expand All @@ -144,14 +152,16 @@
var_line_tainted: false,
var_line_reindented: false,
in_html_comment: false,
multiline_array: false,
multiline_frame: false,
if_block: false,
do_block: false,
do_while: false,
in_case_statement: false, // switch(..){ INSIDE HERE }
in_case: false, // we're on the exact line with "case 0:"
case_body: false, // the indented case-action block
indentation_level: next_indent_level,
line_indent_level: flags_base ? flags_base.line_indent_level : next_indent_level,
start_index: output.length,
ternary_depth: 0
}
return next_flags;
Expand Down Expand Up @@ -207,7 +217,6 @@

last_type = 'TK_START_BLOCK'; // last token type
last_last_text = ''; // pre-last token text
line_indent_level = 0;
output = [];
output_wrapped = false;
output_space_before_token = false;
Expand Down Expand Up @@ -373,10 +382,7 @@
}
}

if (flags.mode === MODE.ArrayLiteral) {
flags.multiline_array = true;
}

flags.multiline_frame = true;

if (!output.length) {
return; // no newline on start of file
Expand All @@ -390,6 +396,8 @@
function print_token_line_indentation() {
if (just_added_newline()) {
if (opt.keep_array_indentation && is_array(flags.mode) && input_wanted_newline) {
// prevent removing of this whitespace as redundant
output.push('');
for (var i = 0; i < whitespace_before_token.length; i += 1) {
output.push(whitespace_before_token[i]);
}
Expand All @@ -408,7 +416,7 @@
function print_indent_string(level) {
// Never indent your first output indent at the start of the file
if (flags.last_text !== '') {
line_indent_level = level;
flags.line_indent_level = level;
for (var i = 0; i < level; i += 1) {
output.push(indent_string);
}
Expand Down Expand Up @@ -443,6 +451,44 @@
flags.indentation_level -= 1;
}

function remove_redundant_indentation(frame) {
// This implementation is effective but has some issues:
// - less than great performance due to array splicing
// - can cause line wrap to happen too soon due to indent removal
// after wrap points are calculated
// These issues are minor compared to ugly indentation.

if(frame.multiline_frame) return;

// remove one indent from each line inside this section
var index = frame.start_index;
while (index <= output.length) {

if (output[index] != '\n') {
index++;
continue;
}

// skip consecutive newlines
while (index <= output.length &&
output[index] == '\n') {
index++;
}

// skip the preindent string if present
if (index <= output.length &&
preindent_string && output[index] == preindent_string) {
index++;
}

// remove one indent, if present
if (index <= output.length &&
output[index] === indent_string) {
output.splice(index, 1);
}
}
}

function set_mode(mode) {
if (flags) {
flag_store.push(flags);
Expand Down Expand Up @@ -919,6 +965,7 @@
// The conditional starts the statement if appropriate.
}

var next_mode = MODE.Expression;
if (token_text === '[') {

if (last_type === 'TK_WORD' || flags.last_text === ')') {
Expand All @@ -927,14 +974,17 @@
if (in_array (flags.last_text, line_starters)) {
output_space_before_token = true;
}
set_mode(MODE.Expression);
set_mode(next_mode);
print_token();
flags.start_index = output.length
indent();
if (opt.space_in_paren) {
output_space_before_token = true;
}
return;
}

next_mode = MODE.ArrayLiteral;
if (is_array(flags.mode)) {
if (flags.last_text === '[' ||
(flags.last_text === ',' && (last_last_text === ']' || last_last_text === '}'))) {
Expand All @@ -948,20 +998,20 @@

} else {
if (flags.last_text === 'for') {
set_mode(MODE.ForInitializer);
next_mode = MODE.ForInitializer;
} else if (in_array (flags.last_text, ['if', 'while'])) {
set_mode(MODE.Conditional);
next_mode = MODE.Conditional;
} else {
set_mode(MODE.Expression);
// next_mode = MODE.Expression;
}
}

if (flags.last_text === ';' || last_type === 'TK_START_BLOCK') {
print_newline();
} else if (last_type === 'TK_END_EXPR' || last_type === 'TK_START_EXPR' || last_type === 'TK_END_BLOCK' || flags.last_text === '.') {
if (input_wanted_newline) {
print_newline();
}
// TODO: Consider whether forcing this is required. Review failing tests when removed.
allow_wrap_or_preserved_newline(input_wanted_newline);
output_wrapped = false;
// do nothing on (( and )( and ][ and ]( and .(
} else if (last_type !== 'TK_WORD' && last_type !== 'TK_OPERATOR') {
output_space_before_token = true;
Expand All @@ -986,13 +1036,12 @@
}
}
}
if (token_text === '[') {
set_mode(MODE.ArrayLiteral);
}

set_mode(next_mode);
print_token();
flags.start_index = output.length
if (opt.space_in_paren) {
output_space_before_token = true;
output_space_before_token = true;
}

// In all cases, if we newline while inside an expression it should be indented.
Expand All @@ -1006,11 +1055,15 @@
restore_mode();
}

if (token_text === ']' && is_array(flags.mode) && flags.multiline_array && !opt.keep_array_indentation) {
if (token_text === ']' && is_array(flags.mode) && flags.multiline_frame && !opt.keep_array_indentation) {
print_newline();
}

if (flags.multiline_frame) {
allow_wrap_or_preserved_newline();
}
if (opt.space_in_paren) {
output_space_before_token = true;
output_space_before_token = true;
}
if (token_text === ']' && opt.keep_array_indentation) {
print_token();
Expand All @@ -1019,6 +1072,7 @@
restore_mode();
print_token();
}
remove_redundant_indentation(previous_flags);

// do {} while () // no statement required after
if (flags.do_while && previous_flags.mode === MODE.Conditional) {
Expand Down Expand Up @@ -1054,7 +1108,7 @@
}
} else {
// if TK_OPERATOR or TK_START_EXPR
if (is_array(previous_flags.mode) && flags.last_text === ',') {
if (is_array(flags.mode) && flags.last_text === ',') {
if (last_last_text === '}') {
// }, { in array context
output_space_before_token = true;
Expand All @@ -1073,7 +1127,6 @@
while (flags.mode === MODE.Statement) {
restore_mode();
}
restore_mode();
var empty_braces = last_type === 'TK_START_BLOCK';

if (opt.brace_style === "expand") {
Expand All @@ -1094,6 +1147,7 @@
}
}
}
restore_mode();
print_token();
}

Expand Down Expand Up @@ -1167,15 +1221,6 @@
print_newline();
}

if (is_expression(flags.mode)) {
// Issue #274
// (function inside expression that is not nested.
if(!(is_expression(flags.parent.mode) || is_array(flags.parent.mode)) &&
line_indent_level < flags.indentation_level) {
deindent();
}
}

print_token();
flags.last_word = token_text;
return;
Expand Down
44 changes: 31 additions & 13 deletions js/test/beautify-tests.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ function run_beautifier_tests(test_obj, Urlencoded, js_beautify)
bt('a=0xff+4', 'a = 0xff + 4');
bt('a = [1, 2, 3, 4]');
bt('F*(g/=f)*g+b', 'F * (g /= f) * g + b');
bt('a.b({c:d})', "a.b({\n c: d\n })");
bt('a.b\n(\n{\nc:\nd\n}\n)', "a.b({\n c: d\n })");
bt('a.b({c:d})', "a.b({\n c: d\n})");
bt('a.b\n(\n{\nc:\nd\n}\n)', "a.b({\n c: d\n})");
bt('a=!b', 'a = !b');
bt('a?b:c', 'a ? b : c');
bt('a?1:2', 'a ? 1 : 2');
Expand Down Expand Up @@ -183,7 +183,7 @@ function run_beautifier_tests(test_obj, Urlencoded, js_beautify)
bt('a = [ // comment\n -1, // comment\n -1, -1\n]');
bt('var a = [ // comment\n -1, // comment\n -1, -1\n]');

bt('o = [{a:b},{c:d}]', 'o = [{\n a: b\n }, {\n c: d\n }\n]');
bt('o = [{a:b},{c:d}]', 'o = [{\n a: b\n}, {\n c: d\n}]');

bt("if (a) {\n do();\n}"); // was: extra space appended

Expand Down Expand Up @@ -237,7 +237,7 @@ function run_beautifier_tests(test_obj, Urlencoded, js_beautify)
test_fragment('/incomplete-regex');

test_fragment('{a:1},{a:2}', '{\n a: 1\n}, {\n a: 2\n}');
test_fragment('var ary=[{a:1}, {a:2}];', 'var ary = [{\n a: 1\n }, {\n a: 2\n }\n];');
test_fragment('var ary=[{a:1}, {a:2}];', 'var ary = [{\n a: 1\n}, {\n a: 2\n}];');

test_fragment('{a:#1', '{\n a: #1'); // incomplete
test_fragment('{a:#', '{\n a: #'); // incomplete
Expand Down Expand Up @@ -433,20 +433,20 @@ function run_beautifier_tests(test_obj, Urlencoded, js_beautify)
bt("a = ['a', 'b', 'c',\n 'd', 'e', 'f',\n 'g', 'h', 'i']",
"a = ['a', 'b', 'c',\n 'd', 'e', 'f',\n 'g', 'h', 'i'\n]");
bt('var x = [{}\n]', 'var x = [{}]');
bt('var x = [{foo:bar}\n]', 'var x = [{\n foo: bar\n }\n]');
bt('var x = [{foo:bar}\n]', 'var x = [{\n foo: bar\n}]');
bt("a = ['something',\n 'completely',\n 'different'];\nif (x);",
"a = ['something',\n 'completely',\n 'different'\n];\nif (x);");
bt("a = ['a','b','c']", "a = ['a', 'b', 'c']");

bt("a = ['a', 'b','c']", "a = ['a', 'b', 'c']");
bt("x = [{'a':0}]",
"x = [{\n 'a': 0\n }\n]");
// this is not great, but is accurate
"x = [{\n 'a': 0\n}]");
bt('{a([[a1]], {b;});}',
'{\n a([\n [a1]\n ], {\n b;\n });\n}');
'{\n a([\n [a1]\n ], {\n b;\n });\n}');
bt("a();\n [\n ['sdfsdfsd'],\n ['sdfsdfsdf']\n ].toString();",
"a();\n[\n ['sdfsdfsd'],\n ['sdfsdfsdf']\n].toString();");
bt("function() {\n Foo([\n ['sdfsdfsd'],\n ['sdfsdfsdf']\n ]);\n}",
"function() {\n Foo([\n ['sdfsdfsd'],\n ['sdfsdfsdf']\n ]);\n}");
"function() {\n Foo([\n ['sdfsdfsd'],\n ['sdfsdfsdf']\n ]);\n}");

opts.keep_array_indentation = true;
bt("a = ['a', 'b', 'c',\n 'd', 'e', 'f']");
Expand All @@ -458,9 +458,9 @@ function run_beautifier_tests(test_obj, Urlencoded, js_beautify)
bt("a = ['a','b','c']", "a = ['a', 'b', 'c']");
bt("a = ['a', 'b','c']", "a = ['a', 'b', 'c']");
bt("x = [{'a':0}]",
"x = [{\n 'a': 0\n }]");
"x = [{\n 'a': 0\n}]");
bt('{a([[a1]], {b;});}',
'{\n a([[a1]], {\n b;\n });\n}');
'{\n a([[a1]], {\n b;\n });\n}');
bt("a();\n [\n ['sdfsdfsd'],\n ['sdfsdfsdf']\n ].toString();",
"a();\n [\n ['sdfsdfsd'],\n ['sdfsdfsdf']\n ].toString();");
bt("function() {\n Foo([\n ['sdfsdfsd'],\n ['sdfsdfsdf']\n ]);\n}",
Expand Down Expand Up @@ -898,7 +898,7 @@ function run_beautifier_tests(test_obj, Urlencoded, js_beautify)
'if (wraps_can_occur && inside_an_if_block) that_is_\n.okay();',
/* expected */
'foo.bar().baz().cucumber((fat &&\n' +
' "sassy") || (leans && mean));\n' +
' "sassy") || (leans && mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap\n' +
' .but_this_can\n' +
'if (wraps_can_occur &&\n' +
Expand Down Expand Up @@ -974,7 +974,7 @@ function run_beautifier_tests(test_obj, Urlencoded, js_beautify)
'if (wraps_can_occur && inside_an_if_block) that_is_\n.okay();',
/* expected */
'foo.bar().baz().cucumber((fat &&\n' +
' "sassy") || (leans && mean));\n' +
' "sassy") || (leans && mean));\n' +
'Test_very_long_variable_name_this_should_never_wrap\n' +
' .but_this_can\n' +
'if (wraps_can_occur &&\n' +
Expand Down Expand Up @@ -1260,6 +1260,24 @@ function run_beautifier_tests(test_obj, Urlencoded, js_beautify)
'})();');
// END tests for issue 268 and 275

// START tests for issue 281
bt('define(["dojo/_base/declare", "my/Employee", "dijit/form/Button",\n' +
' "dojo/_base/lang", "dojo/Deferred"\n' +
'], function(declare, Employee, Button, lang, Deferred) {\n' +
' return declare(Employee, {\n' +
' constructor: function() {\n' +
' new Button({\n' +
' onClick: lang.hitch(this, function() {\n' +
' new Deferred().then(lang.hitch(this, function() {\n' +
' this.salary * 0.25;\n' +
' }));\n' +
' })\n' +
' });\n' +
' }\n' +
' });\n' +
'});');
// END tests for issue 281


// This is what I think these should look like related #256
// we don't have the ability yet
Expand Down

0 comments on commit 7c1ff82

Please sign in to comment.