[Major Bug] v0.10.x does not clean up inline spacing.

[Glavin001]

I just updated coffee-fmt dependency to 0.10.2 for Atom Beautify and it now returns the same text as was inputted, not formatted.

[Glavin001]

I have confirmed that this is applicable for all 0.10.x versions. I am reverting back to 0.7.

[sterpe]

I would need to see an example of this can you send me the file you are testing and the settings you are using.

[Glavin001]

I used your test code: https://github.com/sterpe/coffee-fmt#js-api

var fmt = require('coffee-fmt')
    , fs = require('fs')
    , coffee
    , options
    ;

    options = {
        tab: '\t',
        newLine: '\n'
    };

    //coffee = fs.readFileSync('filename.coffee');
    coffee = "test      ='ing'"; // HARDCODED CHANGE TO TEST 
    coffee = coffee.toString();

    try {
        coffee = fmt.format(coffee, options);
    } catch (e) {
        //Whoops...something went wrong, error details logged to console.
    }

    console.log(coffee);

And then also Atom Beautify's code (which works for 0.7.0 of coffee-fmt): https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/coffee-fmt.coffee#L19-L25

I also tried these two test files: https://github.com/Glavin001/atom-beautify/tree/master/examples/simple-jsbeautifyrc/coffeescript/original

All tests with v0.10.x simply return the same inputted text, without errors.

[sterpe]

Okay so this is just inline spacing issue.

Can you confirm with something like the complexity of this that the tabbing is all okay and that the code isn't broken (shouldn't be, my testing seems fine):

https://github.com/sterpe/coffee-fmt/blob/master/test/global-atom.coffee

0.0.7 will certainly break something like above.

for the most part I was moving to not breaking code in lieue of less transformations...the space thing I can probably clean up a bit more though. Still think you should not revert.

My point being that tab changes seem to be working much better than they were in 0.7.0 where they would break a lot.

[sterpe]

The space thing is complicated, since there is a danger of changing the meaning of the code in certain situations by injecting spaces.

The way I'd like to approach it is to remove any extraneous spaces for the moment, without injecting additional spaces and as time/energy permits complete the parser as necessary to support more robust inline space clean up.

[sterpe]

I guess the other problem is that some people like to tab things out like this in CS:

foo         = 'bar'
x           = 1
y           = 2
etc         = 'you get it'

And it's really unclear how to determine that this is going on, so I was kind of thinking of moving to not interfering with spacing too much in case you were destroying somebody's nice formatting..

I don't know though, what do you think?

[Glavin001]

I guess the other problem is that some people like to tab things out like this in CS:

Good point. In most beautifiers, there are options to select these different behaviours. Personally, I like having all of my extraneous spacing removed and cleaned up, however I have seen that tabbed / lined-up spacing for other developers. An option to switch behaviour would be ideal.

for the most part I was moving to not breaking code in lieue of less transformations...the space thing I can probably clean up a bit more though. Still think you should not revert.

Now I understand. While personally the inline spacing beautification is important for me, I do agree that not breaking the code is more important.

Thanks for explaining. Hopefully this can be resolved soon.

[sterpe]

Yeah I could definitely see keep pretty-print dec/assgn as being a desired option, but extremely difficult to do since pretty much any line is an assignment in coffee script, lol. The language has no concept of an declaration only block.

How about this? I will push out a release in bit that adds removal of any extraneous inline spaces (since those are always unnecessary) and hopefully that get's us near 40-50% of what people at least expect without breaking anything that wasn't already broken, which they have a right to never expect..

Then we can roadmap out a timeline for more robust inline space clean up in future releases.

[Glavin001]

So both of my tests found https://github.com/Glavin001/atom-beautify/tree/master/examples/simple-jsbeautifyrc/coffeescript/original are not being beautified, which we already established is because inline spacing is currently disabled, pending the completion of #13 and #14.

However, now what is being beautified? I notice that in one of my tests:

value          %%=    4000
value     !=        true

does not change at all and the != that was changed to isnt is also no longer working. Both tests essentially do not change, at all.

Given this, I will hold off on updating to v0.10.x until it at least supports the previous behaviour. I can then have another release of Atom Beautify to update coffee-fmt when #13 and #14 is ready.

Thanks!

[sterpe]

I mean these are trivial examples Glavin.

To be clear yes, nothing is being beautified in those two strings.

Can you run something more complex through 0.7.0? I can guarantee that anything with complex structures involving here strings or complex tabbing/scoping is gonna be broken in 0.7.0. I.e. try-catch as you pointed out in a previous issue.

However, 0.10.0+ fixes these things so that if you change your tabination type the code continues to function correctly and begins at a minimum with maintaining working code. Which I imagine is probably a bit more important.

[Glavin001]

I have updated coffee-fmt to v0.10.2 for Atom Beautify and disabled all of the failing tests.

I hope that the inline spacing bug is resolved soon. I can confirm that beautification of larger CoffeeScript tests, more Atom Beautify CoffeeScript code for example, did not break the code.

[sterpe]

v0.11.0 cleans up existing extraneous whitespace (without adding any additional whitespace padding)

ie:

hello   derek how is  it going?
for c, i in "Hello  World!"
  k = 1+1-  2>=3<=  4>5   <6

  for c, i  in "Hello  World"
    k = (a,b)-> if b? then     return a

f  =  b()[0]
for c, i in "Hello World"
  f(b())

beautifies to:

hello derek how is it going?
for c, i in "Hello  World!"
    k = 1+1- 2>=3<= 4>5 <6

    for c, i in "Hello  World"
        k = (a,b)-> if b? then return a

f = b()[0]
for c, i in "Hello World"
    f(b())

Your test above should begin working too...

value %= 4000
value != true

Completed most of the rest of the parser so, we can wrap up inserting whitespace where necessary.

That will come in next release most likely.

[Glavin001]

Thank you!

[sterpe]

Fixed with 2c8ad3be1d14c1e3b21b1b94dabaf3426694fc4b