Possible bug with dot at beginning of line, ternary and object

[tremby]

1.7.0 added "Leading . now closes all open calls, allowing for simpler chaining syntax."

This is great, but I may have just stumbled on a bug with that when using the ternary operator and an object argument.

gulp.task 'stylus', ->
    gulp.src [
        './src/stylus/vars.styl'
        './src/stylus/**/*.styl'
    ]
        .pipe if argv.sourcemaps then require('gulp-sourcemaps').init() else gutil.noop()
        .pipe require('gulp-concat') 'styles.styl'
        .pipe require('gulp-stylus')
            use: require('nib')()
        .pipe if isLocal() then gutil.noop() else require('gulp-minify-css')
            compatibility: '' # Empty string is IE9+ (this is default)
        .pipe if argv.sourcemaps then require('gulp-sourcemaps').write() else gutil.noop()
        .pipe gulp.dest './dist/css'

This is coming out as

// Generated by CoffeeScript 1.9.2
gulp.task('stylus', function() {
  return gulp.src(['./src/stylus/vars.styl', './src/stylus/**/*.styl']).pipe(argv.sourcemaps ? require('gulp-sourcemaps').init() : gutil.noop()).pipe(require('gulp-concat')('styles.styl')).pipe(require('gulp-stylus')({
    use: require('nib')()
  })).pipe(isLocal() ? gutil.noop() : require('gulp-minify-css')({
    compatilibilty: ''
  }).pipe(argv.sourcemaps ? require('gulp-sourcemaps').write() : gutil.noop())).pipe(gulp.dest('./dist/css'));
});

I was expecting

• compatibility: '' object to be an argument to require('gulp-minify-css') -- in this case the result is as expected.
• the dot at the start of the next line to close both the ternary and the pipe() from the preceding line -- in this case the result is not as expected: the next .pipe() is chained on the end of the gulp-minify-css object.

Note that the gulp-stylus pipe above also receives an object as an argument but does not have a ternary. This one compiles as I expect.

Note also that the gulp-sourcemaps pipes have ternaries but no object argument, and these ones also compile as I expect.

So it seems it's a combination of the tenary and the object argument on a new line.

Is this a bug or am I understanding something incorrectly?

[GeoffreyBooth]

There are many ways to confuse the compiler with leading periods. Please use parentheses to make your intentions clear:

gulp.task 'stylus', ->
    gulp.src([
        './src/stylus/vars.styl'
        './src/stylus/**/*.styl'
    ])
    .pipe if argv.sourcemaps then require('gulp-sourcemaps').init() else gutil.noop()
    .pipe require('gulp-concat') 'styles.styl'
    .pipe(require('gulp-stylus')(
        use: require('nib')())
    ).pipe(if isLocal() then gutil.noop() else require('gulp-minify-css')
        compatibility: '' # Empty string is IE9+ (this is default)
    ).pipe if argv.sourcemaps then require('gulp-sourcemaps').write() else gutil.noop()
    .pipe gulp.dest './dist/css'

[tremby]

That's a shame. What's the point in the feature if you can't use it? Or what's the rule on when it can be used and when it can't?

[GeoffreyBooth]

It works well when there aren’t indented lines inside the chaining (use and compatibility in your case). Those seem to confuse the compiler. Perhaps there is a way to disambiguate those, but I get the sense that there isn’t, because this has come up many times before. We probably should’ve disallowed leading period chaining in such scenarios, but it’s been around for so long now that doing so would break lots of code. If I’m wrong and there is a way to make the compiler do what you want, please submit a PR and I’d be happy to accept it.

It’s similar to how sometimes parentheses are required for function calls, for example if you have other code afterward on the same line. When you chain like this, you’re really creating one long line of code; so sometimes parentheses are required to disambiguate.

So to answer your question: use chaining when each line starts with a period. If you need a multiline block in one of the parts of the chain, wrap that block in parentheses (like I did for gulp-stylus above).

[connec]

So it seems it's a combination of the tenary and the object argument on a new line.

I agree. Perhaps the logic for closing implicit calls for leading .s won't continue through ternaries?

[tremby]

Would it help (and make sense? it would to me) to interpret deeper indentation on a following line as a continuation, versus equal (or lesser) indentation as the end of the preceding function call?

Having said that, my own example would break that rule; the first .pipe would then be called on the preceding array, I think. But if this line and succeeding ones were unindented one step it would be clear and unambiguous with that rule, as far as I can see.

[GeoffreyBooth]

Chaining is supposed to be unindented:

$('#button')
.show()
.hide()
.css('color', 'red')
.show()

Which is why it was surprising that your example worked at all.

[tremby]

Does it actually make a difference to the parser at present? If the next line is starting with a ., and is at the same indentation level, or deeper, is there any difference in the interpretation?

I always presumed not, and liked to indent chaining lines as an extra visual clue that they are a continuation of something earlier. (And it always worked for me when I expected it to -- or almost always.) But I have no problem changing the way I write if it would disambiguate and help in keeping code and diffs terse (read: fewer parens).

So perhaps the code example in this thread should now be this:

gulp.task 'stylus', ->
    gulp.src [
        './src/stylus/vars.styl'
        './src/stylus/**/*.styl'
    ]
    .pipe if argv.sourcemaps then require('gulp-sourcemaps').init() else gutil.noop()
    .pipe require('gulp-concat') 'styles.styl'
    .pipe require('gulp-stylus')
        use: require('nib')()
    .pipe if isLocal() then gutil.noop() else require('gulp-minify-css')
        compatibility: '' # Empty string is IE9+ (this is default)
    .pipe if argv.sourcemaps then require('gulp-sourcemaps').write() else gutil.noop()
    .pipe gulp.dest './dist/css'

[lydell]

Chaining is supposed to be unindented

I've never heard about chaining being "supposed" to be in any particular way. All CoffeeScript I've ever written has used indented chaining (because that's what I'm used to from JavaScript). I don't think anyone knows the exact rules. What makes things harder is that the compiler has always been a little too permissive and since CoffeeScript has been around for so long now, it's hard to tell intended behavior from accidental. CoffeeScriptRedux tried to define sane rules for stuff like this, but people have come to depend on accidental quirks (which is one reason people sadly had problems adopting CSR).

[jashkenas]

Chaining like this was not supposed to be supported in the first place. Originally, a leading . on a subsequent line was supposed to to connect to the immediately preceding expression — no closing of implicit parens or braces, period. The point was to avoid the ambiguities.

But that got changed when the PR got merged years ago.

[GeoffreyBooth]

Well that’s what happens when we don’t document a feature 😄

Its only documentation is in the changelog for 1.7.0: http://coffeescript.org/v2/#1.7.0, and there it’s unindented. I had always assumed the unindentation was intentional, since indentation implies a new block, and chaining most emphatically is not a new block—it’s really a single line of code. But clearly the compiler allows either.

We could fix this as part of the breaking changes for 2, but I don’t think we really gain anything other than intellectual cohesiveness, at the expense of lots of people needing to revise their code for no real reason.

[tremby]

But would it help in the context of this thread? Could it make it clear to the compiler what my intended meaning was? See the "I was expecting" list from my original post.

I am increasingly getting the impression that this indentation conversation is actually not relevant to the bug(?) I was pointing out. I think the real issue is that there's something awry when a ternary and object argument are given as part of chains. The dot at the start of the line is supposed to close all preceding function calls, and in this case this is not happening.

[GeoffreyBooth]

Related: #3866.