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

Fix Windows support for upwards recursive .jsbeautifyrc fallback (originally #37) #85

Closed
BlaM opened this issue Sep 10, 2014 · 19 comments
Closed
Assignees
Labels

Comments

@BlaM
Copy link

BlaM commented Sep 10, 2014

Not sure what I'm doing wrong, but all this package seems to do on the two Windows machines I tried it on is to lock up Atom.

As soon as I press ALT+CTRL+B the whole editor window freezes and after some time a window pops up telling me that something got stuck and asks me if I want to continue waiting or if I want to halt it. Waiting seems to go on forever.

Atom v0.125.0
Windows 7 64bit - one with English locals, one with German locals.

Tried with HTML and JS, both with the same result.

@Glavin001
Copy link
Owner

Can you also confirm what version of Atom Beautify you are running?

@Glavin001 Glavin001 added the bug label Sep 10, 2014
@Glavin001 Glavin001 self-assigned this Sep 10, 2014
@BlaM
Copy link
Author

BlaM commented Sep 10, 2014

I tried with versions 0.7.0 (via install last week) and 0.10.0 (via update today).

@BlaM
Copy link
Author

BlaM commented Sep 10, 2014

Not sure if it's noteworthy: I installed them through the "Packages" interface in Settings, not via command-line "apm".

@Glavin001
Copy link
Owner

And 0.10.0 same issue? Strange. I have not seen this before. I will try 0.10.0 and a few different laptops I have and see if I can reproduce. Thanks!

@BlaM
Copy link
Author

BlaM commented Sep 10, 2014

Just to be sure I tried a clean reinstall (completely remove package before installing it again), but with the same result: Locking up - and yes, also with 0.10.0.

I attempted to reformat a 40 line JS file.

Is there anything I can do to gather more info to help you?

@Glavin001
Copy link
Owner

Including the 40 line JS file contents here so I could test with it would be ideal, however that is up to you if you're in a position to share that source code.

@BlaM
Copy link
Author

BlaM commented Sep 10, 2014

The JS file does not make a difference. It also happens with any other file. I just confirmed with this one:
https://github.com/h5bp/html5-boilerplate/blob/master/src/js/plugins.js

@Glavin001
Copy link
Owner

Could you check your JavaScript console in Developer Tools?

@BlaM
Copy link
Author

BlaM commented Sep 10, 2014

I can open it and I can find beautify.coffee in there. I can have a look at whatever you like if you can give me specific instructions - but so far I don't have the time (or - to be honest - the motivation) to learn CoffeeScript.

@Glavin001
Copy link
Owner

I want you to just check for errors in the log.

@BlaM
Copy link
Author

BlaM commented Sep 10, 2014

Okay, I singlestepped through it for a while and now it seems to be stuck in a bit of code here:
https://github.com/Glavin001/atom-beautify/blob/master/lib/beautify.coffee#L252

Not absolutely sure what it does, but my guess is that it starts at some directory and then traverses up until it reaches "/" (root). However in my case (Windows) that never happens, because my root currently is "T:" (that's the content of that variable p).

@RangerMauve
Copy link

I've also had this issue. But I only noticed it when I updated to 0.10.0. (Windows 8)

@Glavin001
Copy link
Owner

Oh, of course! That absolutely makes sense. Thank you for diagnosing this issue @BlaM. I don't have a Windows machine to test on so I neglected to think of that case. Will get right on it.

@Glavin001
Copy link
Owner

Could you change:

while p isnt "/"

to:

while p isnt path.resolve(p,"../")

And let me know if that works.

@RangerMauve
Copy link

It gets up to C:\ and all subsequent calls to path.resolve return the same value.

Maybe count the number of path.sep in the string and return when it's only one?

Probably with this:

while p.split(path.sep).length isnt 2

@Glavin001
Copy link
Owner

It gets up to C:\ and all subsequent calls to path.resolve return the same value.

Exactly.

So,

while p isnt path.resolve(p,"../")

should equate to

while "C:\" isnt "C:\"

which of course will fail and stop the while loop.

Did it not work?

@RangerMauve
Copy link

Sorry, I made a mistake recreating a minimal example. It works just as expected.

@Glavin001
Copy link
Owner

Okay, if that is confirmed working then I will publish a patch.

@Glavin001 Glavin001 changed the title Locking up Fix Windows support for upwards recursive .jsbeautifyrc fallback (originally #37) Sep 10, 2014
@Glavin001
Copy link
Owner

Published to v0.10.2

Thanks for your patient and support everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants