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

break chain method #161

Closed
bigyuki opened this issue Sep 9, 2015 · 9 comments
Closed

break chain method #161

bigyuki opened this issue Sep 9, 2015 · 9 comments

Comments

@bigyuki
Copy link

bigyuki commented Sep 9, 2015

before:

gulp.task('webpack', ['clean'], function () {
    return gulp.src(paths.scripts)
        .pipe(webpack(webpackConfig))
        .pipe(uglify())
        .pipe(gulp.dest(paths.scriptsDest));
});

after:

gulp.task('webpack', ['clean'], function () {
    return gulp.src(paths.scripts).pipe(webpack(webpackConfig)).pipe(uglify()).pipe(gulp.dest(paths.scriptsDest));
});

this is not i want.

@prettydiff
Copy link
Owner

You can test it out now. http://prettydiff.com/ignore/testlocation/prettydiff.com.xhtml

Just let me know if the code looks better or weird things happen to your code. I made this the default and provided an option methodchain for users who want to force chains of methods onto a single line.

Some fair warning in that the code is looking for a . (period) following a ) (closing paren) instead of looking to verify if its actual chains of methods.

@prettydiff prettydiff added this to the v1.13.6 milestone Sep 9, 2015
@prettydiff
Copy link
Owner

Hold on and don't test yet. I am getting some serious regression from this.

@prettydiff
Copy link
Owner

Patterns I am thinking about:

//split on properties as well
var a = location.href.toLowerCase().split("/")[3].toUpperCase().split("%20");
var a = location
    .href
    .toLowerCase() //ensure comments in the middle don't break things
    .split("/")[3]
    .toUpperCase()
    .split("%20");

//splitting methods inside a higher method looks odd
var b = func(myMethod().property.otherMethod() > 5 && somethingElse + other[5] < item[b][c[d]]);
var b = func(myMethod()
    .property
    .otherMethod() > 5 && somethingElse + other[5] < item[b][c[d]]);

//I need to figure out some minimal limitations, because this is a bit ridiculous
var c = node().value;
var c = node()
    .value;

//no method, no fancy prize
var d = cat.dog.pony.show.inthe.town;

//only final item is a method.... no change
var e = cat.dog.pony.show.inthe.town();

//very strange, notice two separate chains separated by operators and logic
var f = func(myMethod().property.otherMethod() > 5 && somethingElse + other[5] < item[b][c[d]].cat.dog.very.strange);
var f = func(myMethod()
    .property
    .otherMethod() > 5 && somethingElse + other[5] < item[b][c[d]]
    .cat
    .dog
    .very
    .strange);

@prettydiff
Copy link
Owner

Now you should be able to test away: You can test it out now. http://prettydiff.com/ignore/testlocation/prettydiff.com.xhtml

@prettydiff
Copy link
Owner

Testing against JSLint. Must look more at lists

function q() {
    "use strict";
    var all = 0,
        value = 0,
        auto = 0,
        pd = 0,
        dateList = 0,
        componentArea = 0,
        lang = 0,
        table = 0,
        token = 0,
        a = 0;
    if (all === true && lang === "") {
        value = auto(pd.ace.beauIn.getValue());
        pd.o.langvalue = value;
    }
    all
        .ace
        .beauIn
        .getSession()
        .setMode("ace/mode/" + value[0]);
    dateList = dateList
        .sort(function dom__load_sort_forward(componentArea, row) {
            return componentArea[1] === row[1];
        })
        .reverse()
        .sort(function dom__load_sort_reverse(componentArea, row) {
            return componentArea[0] - row[0];
        });
    table.push(token[a]
        .replace(/&/g, "&amp;")
        .replace(/</g, "&lt;")
        .replace(/>/g, "&gt;"));
    return [
        all
            .join("")
            .replace(all, "<pd>")
            .replace(all, "</pd>"),
        all
            .join("")
            .replace(all, "<pd>")
            .replace(all, "</pd>")
    ];
}

@bigyuki
Copy link
Author

bigyuki commented Sep 10, 2015

@prettydiff Hi, I test my code, is ok, thank you.

@prettydiff
Copy link
Owner

I am looking at what to do with the following code.

var c,
    k = function q(u
            .big()
            .method,
        j
            .small()
            .ethod()) {
        a
            .big()
            .method;
        b
            .small()
            .ethod;
    };

I am probably going to scale this feature back so that it is never applied inside ( and ) type containers.

@prettydiff
Copy link
Owner

var a,
    result = (function jspretty__resultScope() {
        for () {}
        build = [
                "</em> unnecessary instances of the keyword <strong>new</strong> counted.</p>", data, last
            ];
        }()).replace(/(\s+)$/, "")
        .replace(/\u009f/g, "");
    };

Found a problem with functions

@prettydiff
Copy link
Owner

I have this all squared away now and I plan to launch tonight. This enhancement will come with these limitations:

  • Method chains originating from a } will not be broken. This just looks weird to read.
  • Method chains whose immediate container is parenthesis will not be broken. Breaking here makes the code very confusing to figure out.
  • Indentation inside functions, such as functions wrapped in parenthesis, will not be altered so long as the method chain originates from something directly wrapping the function.
  • The first property/method in the method chain will not be broken. This visually indicates that a property is directly riding on something and that following (line broken) properties are associated (and indented), as in a chain.
  • The prior rule is discarded if the first property in the chain is a method containing a function as an argument directly.

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

2 participants