millermedeiros / esformatter Goto Github PK

It would be great if we had a --help parameter that shows usage of esformatter CLI tool:

$ esformatter --help

Related to the discussions in #16, but that seems a bad place to continue that discussion.

You've expressed interest there to provide a jQuery preset. In that regard, I just landed an update to that style guide that gets rid of ~three exceptions. Its still has some, but at least there's less of them.

Current version: http://contribute.jquery.org/style-guide/js/
Changes: https://github.com/jquery/contribute.jquery.org/pull/22/files

I would like to be able to specify a configuration file to use when running in CLI mode, such as:

$ esformatter --config myconfig.json

In the jQuery Style guide, variable declarations without init go on one line. For example:

// Good
var a, b, c,
    foo = true,
    bar = false;

This needs its own config to implement #19.

input:

var obj = {
    then: function( /* fnDone, fnFail, fnProgress */ ) {
        var fns = arguments;
    }
};

output:

var obj = {
    then : function(    /* fnDone, fnFail, fnProgress */) {
        var fns = arguments;
    }
};

I write a lot of code in ExtJS, and it's desirable to format configs in this manner:

Ext.define('VMS.model.User', {
  extend: 'Ext.data.Model',
  idProperty: 'UserID',
  fields: [
    {
      name: 'UserID',
      type: 'int'
    },
    {
      name: 'FirstName',
      type: 'string'
    },
    {
      name: 'LastName',
      type: 'string'
    }
  ]
});

However, I was unable to find an option to add line breaks in this manner (how the fields array is formatted). This would be a great addition to EsFormatter.

If I have

function foo(a){
  return
    a<5?23:12;
}

I get:

function foo(a) {
    return    a < 5 ? 23 : 12;
}

Instead, it should be:

function foo(a) {
    return a < 5 ? 23 : 12;
}

Which option do I need to change to force an opening curly brace to be on the same line as the appropriate statement / expression?

E.g.:

var add = function(a, b)
{
    return a + b;
}

should become

var add = function(a, b) {
    return a + b;
}

Shouldn't the dependency of amd-utils be changed to mout, since mout is more actively updated now?

input:

// from jquery
jQuery.prototype = {
    get: function( num ) {
        return num == null ?

            // Return a 'clean' array
            this.toArray() :

            // Return just the object
            ( num < 0 ? this[ this.length + num ] : this[ num ] );
    }
};

output:

// from jquery
jQuery.prototype = {
        get : function(num) {
            return num == null ?             // Return a 'clean' array
this.toArray() :             // Return just the object
(num < 0 ? this[this.length + num] : this[num]);
        }
};

it should keep original line break if that is the case.

I think we should augment all the token methods to also accept a string (instead of a callback) for brevity, so this block:

start = _tk.findInBetween(start, end, function(token) {
    return token.value === '(';
});

could be rewritten as:

start = _tk.findInBetween(start, end, '(');

right now the string only checks for token.type, but I think it should also fallback to token.value to make it simpler.

Format an AST

At the moment the format function only accepts a string.
It would be nice to support a rocambole AST. In my use case, I use rocambole to fetch the AST with all those extra tokens, modify it, and want be able to format it.

Don't reformat code

Assuming that the feature request above is done, I would like esformatter to be able to take an AST and output the correspondent source code WITHOUT formatting. I don't know if this gets out of the scope of this tool.

input:

jQuery.ready.promise = function( obj ) {
    try {
        top = window.frameElement == null && document.documentElement;
    } catch(e) {}
};

output:

jQuery.ready.promise = function( obj ) {
    try {
        top = window.frameElement == null && document.documentElement;
} catch (e) {
}
};

also notice that empty catch block should probably be on a single line.

If I have

var x=33
   ,y=12;

it should get:

var x = 33,
    y = 12;

Instead I get:

var x = 33
,
    y = 12;

this should be indented properly:

returned.promise()
.done(newDefer.resolve)
.fail(newDefer.reject)
.progress(newDefer.notify);

dumb code duplication is dumb :P

_ws.aroundIfNeeded(node.body.startToken, 'FunctionDeclarationOpeningBrace');
_ws.aroundIfNeeded(node.body.endToken, 'FunctionDeclarationClosingBrace');

_br.aroundIfNeeded(node.body.startToken, 'FunctionDeclarationOpeningBrace');
_br.aroundIfNeeded(node.body.endToken, 'FunctionDeclarationClosingBrace');

could be written as:

_insert.aroundIfNeeded(node.body.startToken. 'FunctionDeclarationOpeningBrace');
_insert.aroundIfNeeded(node.body.endToken. 'FunctionDeclarationClosingBrace');

this is repeated on many hooks.

right now I remove trailing whitespace and indentation before processing the tokens (using RegExp) I think a better approach would be to convert the simple WhiteSpace tokens that are at the start of the line to an Indent token (without changing the amount of chars or anything like that - just set the token.type) and make our indentation code simply replace the amount of chars (or add a new token if it doesn't exist).

That will probably make it easier to implement #10 and #12 and will also make it easier to keep user indentation (if needed).

this might be tricky to implement specially since it might not be the desired behavior in all cases:

var tuples = [
    // action, add listener, listener list, final state
    ["resolve", "done", jQuery.Callbacks("once memory"), "resolved"],
    ["reject", "fail", jQuery.Callbacks("once memory"), "rejected"],
    ["notify", "progress", jQuery.Callbacks("memory")]
];

probably do it only if user added line breaks.

I would like to be able to specify an input file using a CLI parameter, such as:

$ esformatter myscript.js

integers are more flexible since in some cases I think we should have 3+ states. I'm thinking about a setting that keeps the original white spaces (for cases where the user added the spaces to manually align some tokens - think about tabular data) so we should have a way to set if it should remove the existing white space, keep it or add if it doesn't exist, example:

// remove if it exists (consecutive tokens) and don't add if it's missing
var REMOVE = 0;
// add if missing or keep a single one 
var ADD_IF_MISSING = 1;
// add if missing or keep if it already exists (no matter how many)
var ADD_OR_KEEP = 2;
// keep exact amount (remove if more, add if less)
var EXACT = { count : n };
// don't change
var NOOP = 4;

var DEFAULT_OPTS = {
  lineBreak : {
    before : {
      // pass an object for advanced settings
      // this will keep 3 line breaks (add if missing)
      FunctionDeclaration : {
        count : 3
      }
    },
    after : {
      FunctionDeclarationOpeningBrace : ADD_OR_KEEP
    }
  }
}

Of course this will increase the complexity a lot and will take considerably more time. This is definitely not a priority for now.

When invoked after a pipe, esformatter seems to work fine. However, invoking the help message via -h or --help causes it to hang and the prompt does not return. Installed esformatter via npm install -g esformatter on a RHEL Linux x86_64 machine.

Unless the test input specifies otherwise, all tests should automatically take their expected input and wrap it in a statement block:

[1,2,3]

{
    statementBefore();
    [1,2,3];
    statementAfter();
}

Most scenarios should remain unchanged except for being indented one layer deeper and it will add one degree of automatic breadth to the test matrix.

jQuery.guid++ should not be indented:

proxy.guid = fn.guid = fn.guid ||             jQuery.guid++;

this is wrong, jQuery.readyWait should not be indented:

if (wait === true ? --                jQuery.readyWait : jQuery.isReady) {
    return;
}

the indent should affect all lines of the comment block not just the first line.

it will require a different logic, need to edit the comment.raw value directly and it is only possible to do it after #82 gets merged (needs original indentation to make it work properly).

I already have a lot of ideas on how I would like things to work but I should communicate them better with possible contributors through a roadmap (explaining planned features and how far we are from reaching the goal). Maybe that can motivate more contributors.

input:

function isPlainObject(){
    // Not own constructor property must be Object
    if ( obj.constructor &&
        !core_hasOwn.call(obj, "constructor") &&
        !core_hasOwn.call(obj.constructor.prototype, "isPrototypeOf") ) {
        return false;
    }
}

output:

function isPlainObject(){
    // Not own constructor property must be Object
    if (obj.constructor &&
!core_hasOwn.call(obj, "constructor") &&
!core_hasOwn.call(obj.constructor.prototype, "isPrototypeOf")) {
        return false;
    }
}

I think it should keep the line breaks and just indent each line.

It would be great if you could run esformatter as part of grunt using a grunt plugin.

If I change the ArgumentList from 0 to 1 in whiteSpace before:

Error: 
/Users/satazor/Work/indigo/dejavu/node_modules/esformatter/lib/util/token.js:14
        var startRange = target.range[0];
                               ^
TypeError: Cannot read property 'range' of undefined
    at Object.before (/Users/satazor/Work/indigo/dejavu/node_modules/esformatter/lib/util/token.js:14:32)

There are a lot of rules and not all of them are easy to grasp.

Codepainter has this useful feature of inferring the style from a user supplied code sample. I think that would make esformatter a lot more approachable and easier to use.

expected:

var 
     // foo var
     foo,
     // bar variable
     bar = 'dolor amet';

actual:

var    // foo varfoo,
// bar variable
     bar = 'dolor amet';

Trying esformatter for the first time, I didn't make it far. I did an npm install esformatter, then saved this as test.js and ran it with node:

var esformatter = require( "esformatter" ),
    fs = require( "fs" ),
    src = fs.readFileSync( "test.js", "utf-8" ),
    formatted = esformatter.format( src );
console.log( formatted );

Results in this stacktrace:

$ node test.js

[...]/node_modules/esformatter/lib/util/token.js:93
        var lineDiff = target.loc.start.line - target.loc.end.line;
                                 ^
TypeError: Cannot read property 'start' of undefined
    at Object.remove (/Users/jza/dev/jquery-ui/node_modules/esformatter/lib/util/token.js:93:34)
    at Object.sanitizeWhiteSpaces (/Users/jza/dev/jquery-ui/node_modules/esformatter/lib/util/whiteSpace.js:148:17)
    at Object.format (/Users/jza/dev/jquery-ui/node_modules/esformatter/lib/esformatter.js:42:9)
    at Object.<anonymous> (/Users/jza/dev/jquery-ui/test.js:4:25)
    at Module._compile (module.js:449:26)
    at Object.Module._extensions..js (module.js:467:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.runMain (module.js:492:10)
    at process.startup.processNextTick.process._tickCallback (node.js:244:9)

Some basic usage instructions in the readme would be nice, in case that's not how I'm supposed to use it. While at that, I didn't yet see anything for actually configuring options, but then I haven't looked very far either.

input:

(function(){
var
    // A central reference to the root jQuery(document)
    rootjQuery,

    // Define a local copy of jQuery
    jQuery = function( selector, context ) {
        // The jQuery object is actually just the init constructor 'enhanced'
        return new jQuery.fn.init( selector, context, rootjQuery );
    };
}());

output:

(function(){
    var
            // A central reference to the root jQuery(document)
        rootjQuery,

            // Define a local copy of jQuery
        jQuery = function(selector, context) {
                // The jQuery object is actually just the init constructor 'enhanced'
        return new jQuery.fn.init(selector, context, rootjQuery);
    };
}());

might be related to #30

implement it!

One of the ways similar tools get immediate exposure and traction is by having a web page showing off their basic functionality. For example, https://github.com/einars/js-beautify has http://jsbeautifier.org/ .

Github makes this easy to do using a gh-pages branch.

this is wrong:

for (i in {                    submit : true,                    change : true,                    focusin : true            }) {
  ...
}

input:

// Own properties are enumerated firstly, so to speed up,
// if last one is own, then all properties are own.

var key;
for ( key in obj ) {}

output:

// Own properties are enumerated firstly, so to speed up,
// if last one is own, then all properties are own.

var key;
for (key in obj) {
}

It would be nice to configure the formatting of switch blocks

Input

switch(day){
    case 0:
        x = "Today it's Sunday";
        break;
    case 1: case 2:
        x = "Today it's Monday";
        break;
}

Output

switch(day){
case 0:
x = "Today it's Sunday";
break;
case 1:case 2:
x = "Today it's Monday";
break;
}

The options likely to be configured are:

• space after switch
• space or newline before or after curly bracket
• newline after break

case 0:
    break;

case 1:

• newline after fallthrough

case 0:
case 1:

• Indentation

switch (day) {
    case 0:
        x = 1;
        break;

or

switch (day) {
case 0:
    x = 1;
    break;

or

switch (day) {
    case 0:
        x = 1;
    break;

or many others

Looking at https://github.com/millermedeiros/esformatter/blob/master/test/format.spec.js at these lines 2432db5#L0R38 , I notice that it does not run the resulting output through the formatter again to ensure consistent results.

Jsbeautifier has seen several bugs where output would flip flop between two states or otherwise change with multiple executions. That project works with incomplete parsing so it is more likely to encounter this problem, but with sufficiently complex rules this class of issue is worth covering.

From test/compare/default/README.dm:

The default settings should be as conservative as possible, Google JavaScript Style Guide should be used as a reference.

For object expressions, Google doesn't have a space in front of the colon:

// Object initializer.
var inset = {
  top: 10,
  right: 20,
  bottom: 15,
  left: 12
};

Following that, the default for whiteSpace.after.PropertyName should be 0, not 1.

input:

            if ( window.DOMParser ) { // Standard
                tmp = new DOMParser();
                xml = tmp.parseFromString( data , "text/xml" );
            } else { // IE
                xml = new ActiveXObject( "Microsoft.XMLDOM" );
                xml.async = "false";
                xml.loadXML( data );
            }

output:

                if (window.DOMParser) {
// Standard
                    tmp = new DOMParser();
                    xml = tmp.parseFromString(data, "text/xml");
                } else {
// IE
                    xml = new ActiveXObject("Microsoft.XMLDOM");
                    xml.async = "false";
                    xml.loadXML(data);
                }

I think the best place to do it is on _br.afterIfNeeded() since it will probably fix this issue in multiple places at once.

Input/expected:

call(function(y) {
    x();
});

Output/actual:

call(function(y){
    x();
});

The space in front of { is missing.

If I run esformatter against

function add(a,b)
{
return a+b;
}


function sub(a,b)
{ return a-b;}

I get

function add(a, b) {
    return a + b;
}


function sub(a, b) {
    return a - b;
}

Shouldn't it be:

function add(a, b) {
    return a + b;
}

function sub(a, b) {
    return a - b;
}

(Only one single empty line according to keepEmptyLines: 1 instead of two lines?)

did a "quick" (not as quick as I expected) test using jQuery 1.8.3 and it takes forever to process the file.

half of the reason is because rocambole is too slow (see millermedeiros/rocambole#4), the other half of the time is spend on sanitizeWhiteSpaces and executing the hooks for each node.

I added a few console.time and console.timeEnd calls, here is the output:

opts     : 2ms
indent   : 2ms
trail    : 1ms
empty    : 0ms
parse    : 184354ms
sanitize : 61622ms
moonwalk : 119392ms
toString : 20ms
TOTAL    : 365393ms (aka. forever)

Still need to profile it better (only did the console.time so far) but I know I'm doing a lot of unnecessary work (lots of function calls and removing/inserting unnecessary tokens).

I hope to bring the total time below 5 seconds (ideally under 3s), lets see how it goes.

Make it right before you make it faster.

PS: I will probably "finish" the support for all statements before working on the performance.

Hello,
do you know if anyone is working on a SublimeText plugin for formatting code using esformatter?

I really need it and I can work on it, but I'm new to Python, so I just want to be sure that I don't waste time if someone else is doing it already.

I've been trying for a while now to add a config option to add more whitespace inside parenthesis, specifically for rather generic expressions. These appear on ExpressionStatements and sometimes BinaryStatements. Since the former can have many forms and parenthesis are, kind of like comments, not a primary citizen, this turns out to be pretty difficult.

My test file looks like this:

;( ++n < 10 );
;( a == b );
( a === b );
x = ( value / 10 );
if (x * ( x + 1 ) + ( y + 2 )) {
    y = ( "123" + "3456" );
}

I haven't yet been able to find a solution for this. My current experiment is on a branch here: jzaefferer@77c30ed
I ended up applying the same logic to BinaryStatement as for ExpressionStatement. This actually gets pretty close, for my limited test at least:

It'll probably mess up other code though, since so many things are ExpressionStatements.

I should fork mocha and use the absolute path to my fork so users can simply do a npm install --dev.

I think we should have presets for the most common styles:

• http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml
• http://nodeguide.com/style.html
• https://github.com/rwldrn/idiomatic.js/

also do it in a way that user can store their settings on the project folder or as a global file.

Input:

(function( window, undefined ) {
var
    // A central reference to the root jQuery(document)
    rootjQuery,

    // The deferred used on DOM ready
    readyList;
}());

Output:

(function(window, undefined){
    var
            // A central reference to the root jQuery(document)
        rootjQuery,

            // The deferred used on DOM ready
        readyList;
}());

Comment indent should be aligned with the var declarations.

expected:

doc = (context && context.nodeType ? context.ownerDocument || context : document);

actual:

doc =( context && context.nodeType ? context.ownerDocument || context : document);

maybe try to handle it automatically inside _ws.beforeIfNeeded.