Support newline before logical or ternary operator

[olsonpm]

So I use ternary operators every once in a while, and I format them like the following:

var result = (user === myUser)
  ? myUser
  : defaultUser;

however running this through js-beautify I get

var result = (user === myUser) ? myUser : defaultUser;

I haven't used the tool much, and didn't see any relevant options besides the preserve lines but that didn't help. I understand this is a very narrow use-case and so if the functionality doesn't exist I'm fine with that.

Thanks for the work put into this library

[olsonpm]

Oo, I just realized this made my big equality function a crazy one-liner

function equals(other) {
  return this.prop1 === other.prop1
    && this.prop2 === other.prop2
    && this.prop3 === other.prop3;
}

turns into

function equals(other) {
  return this.prop1 === other.prop1 && this.prop2 === other.prop2 && this.prop3 === other.prop3;
}

So this is more far reaching than I first thought. I'll take a look at the source.

[olsonpm]

Congratulations on an easy-to read library.

In the small chance that future readers use the same formatting I do, the following code snippet should solve your problems. I haven't tested much besides the few files I wanted js-beautify for, but I will update it if I run into any issues.

Inserted at line 1079 of js-beautify version 1.5.4

// Allow this kind of newline preservation:
// a = b
//   && (c || d);
//
// and also this kind of newline preservation:
// a = (b === c)
//   ? d
//   : e;
if (current_token.text === '&&' || current_token.text === '||' || current_token.text === '?' || current_token.text === ':') {
    if (last_type === 'TK_WORD' || flags.last_text === ')') {
        if (!start_of_object_property()) {
            allow_wrap_or_preserved_newline();
        }
    }
}

due to my styling be different from most others, I won't bother with a PR. Go ahead and close this.

[bitwiseman]

No, your suggestion seems very reasonable.
It would be great if you'd be willing to add this to the javascript and python, and a few tests for this.
If not, at least we have the example to work from later.
Thanks!

[olsonpm]

I appreciate the feedback. I just looked into running the /js/tests/run-tests and found it requires /usr/bin/spidermonkey-1.7 to exist. Any documentation on that? I searched the repo and found nothing.

[bitwiseman]

try running make all. I don't think I have spidermonkey on my system and I build and test just fine.

[olsonpm]

I'll have a fix finished tonight, but man would it be helpful for all these tests to be associated with some id for quick reference haha

[olsonpm]

And actually I do need your or another maintainer's input on whether this should be optional behavior. The only time I can see where it matters is the following case:

var a = (b || c);

Without an option, the above case is ambiguous. As I see it, we can either 1) Make operator-first style an option 2) Decide on a default when an ambiguous case arises.

I lean towards # 2 because I don't feel any ambiguous cases will be important enough to style correctly. If someone writes code like the above, they likely don't care much about how it comes out.

Please let me know your thoughts.

[bitwiseman]

Ah, I see.

It makes sense to do #1 in order to preserve existing behavior - no one is going to complain, that your change broke them. It also keeps the combinations for testing somewhat simpler...

But I can see the attraction of #2. As a default, you should be able to check if the operator is also followed by a new line, then force to one behavior or the other. They may not care, but we should shoot for some consistency.

[olsonpm]

I don't mean to be argumentative - but I don't see a case where anyone will complain about their code breaking as a result of this new operator-first change. I agree that it would be safer to make this optional, but mainly because I understand that I can't foresee all use-cases.

I will certainly make it an option, but for sake of playing devil's advocate, can you think of a case where someone would be surprised?

Edit: I will have a PR tomorrow with it being an option :)

[bitwiseman]

You'd be surprised. :smile: Let's say someone is expecting the beautifier to force operator-before-newline and then it doesn't anymore, as in the case of team-wide style enforcer.

[olsonpm]

Ah good point - Thanks much

[olsonpm]

I find it funny the beautify.js isn't beautified haha.

relevant pull request

The code I wrote could easily be expanded to allow for other operator-first notation (such as string concatenation). Just food for thought.

[alexjamesbrown]

did this ever get solved?

[olsonpm]

Nope. I let it be because interest doesn't seem to be there. Taking a look at your referenced issue however, I'd love to have a maintainer give me feedback on my PR. I'm willing to add tests/modify the code as needed.

[olsonpm]

@alexjamesbrown - I'll for sure have the PR updated by mid-next week, though I'm aiming for this weekend. Go ahead and watch the relevent PR for further updates. When the PR is merged, I'll mark your referenced issue as closed.

[alexjamesbrown]

cool, sounds good! thanks!

[ghost]

I would love newline option for && || ? : operators

[olsonpm]

Ha - I know, bitwiseman and I were working on a draft and life caught up with us. These hollidays are killing me along with work. I will get to this when I can. I appreciate the interest though - because it's nice to know people will use this feature.

[ghost]

Also looking for an option to beautify our busy schedules :smiley:

[bitwiseman]

735 added option to control this.