Glavin001 / atom-beautify

:mega: Help Wanted - Looking for Maintainer: https://github.com/Glavin001/atom-beautify/issues/2572 | :lipstick: Universal beautification package for Atom editor (:warning: Currently migrating to https://github.com/Unibeautify/ and have very limited bandwidth for Atom-Beautify Issues. Thank you for your patience and understanding :heart: )
http://unibeautify.com/
MIT License
1.5k stars 453 forks source link

Won't beautify HTML any more, and settings dialog is broken #898

Closed derwaldgeist closed 8 years ago

derwaldgeist commented 8 years ago

Description

Since the last update, no HTML is being beautified any more. If I call atom-beautify manually (from the menu), I get a message that some settings are unsupported. If I look at the settings dialog, it looks quite broken - only a few settings are enabled, and some settings (like Analytics User Id) show up twice.

Expected Results

Working settings dialog, working HTML beautification.

Steps to Reproduce

  1. Just update to the latest version
  2. Open an HTML file
  3. Run command Atom Beautify: Beautify Editor
  4. An error message appears (yellow)
Glavin001 commented 8 years ago

This is the aftermath of major (breaking) changes to Atom-Beautify's settings: https://github.com/Glavin001/atom-beautify/pull/864

It should tell you which options are no longer supported and also say Please run Atom command 'Atom-Beautify: Migrate Settings'.: https://github.com/Glavin001/atom-beautify/blob/master/src/beautify.coffee#L522-L525

    atom.notifications.addWarning("You have unsupported options: #{unsupportedOptions.join(', ')} <br> Please run Atom command 'Atom-Beautify: Migrate Settings'.")

Does it not tell you to Please run Atom command 'Atom-Beautify: Migrate Settings' in the warning message? Have you already run Migrate Settings? Did it not work?

derwaldgeist commented 8 years ago

Thanks for your quick reply. I found out that it it shows that message, but it is not visible on screen if you have more than a couple of incompatible options. Because the message disappears after some seconds, you also have nearly no chance to scroll down the window. I only found this out when I tried the debug command - in this case, I could have a closer look at the message. So I would recommend you to place the message at the top or even do the conversion silently in the background.

Anyways, after I ran the migration, I had to re-tweak nearly all options myself because they had been cleared. For instance, all (or nearly all?) beautify on save options were lost. Plus, I wasn't able to recover the previous state completely. One sample: in SCSS, I had newlines before every media query. Now these newlines are removed, despite of the setting to add a new line between selectors. Preserving newlines also doesn't work as expected.

BTW: Thanks for this great package. I can't develop without it.

prettydiff commented 8 years ago

@derwaldgeist The default beautifier for SCSS is prettydiff. If you could test your code at http://prettydiff.com/?m=beautify to determine if this is a Pretty Diff bug instead of an Atom Beautify bug.

Glavin001 commented 8 years ago

So I would recommend you to place the message at the top or even do the conversion silently in the background.

Thanks for letting me know! I will move it to the top of the warning. I was thinking of automatically migrating the options, I just worry about doing things automatically for users. I suppose there will be problems either way, with this latest breaking change.

after I ran the migration, I had to re-tweak nearly all options myself because they had been cleared.

Noo! I even wrote tests to verify the Migration tool worked. Do you have specific options you could mention that did not migrate properly? I will add explicit tests for those.

For instance, all (or nearly all?) beautify on save options were lost.

In my haste, I missed writing tests for the Language Config options which are now organized under each language, as you have seen now. It makes sense that this is failing, now that I think about it. This is unfortunate.

One sample: in SCSS, I had newlines before every media query. Now these newlines are removed, despite of the setting to add a new line between selectors. Preserving newlines also doesn't work as expected.

These options should have migrated properly and be picked up properly. Could you provide another debug.md Gist demonstrating that this is broken? Thanks in advance!

I apologize for these problems. It was a large release in order to bring forth a greatly improved UI, but at a cost of breaking changes. I had hoped the Migration tool would help, however I rushed it a little bit and missed testing a few cases. Thank you for your feedback and I'll try to resolve these issues so that other users can have a smooth upgrade experience.

Glavin001 commented 8 years ago

After adding more tests, it appears that the Language Config options do actually pass the tests (the RegExp used for conversion was flexible enough to handle it). So I am not sure why the migration did not work for you. Any more information would be greatly appreciated. Thank you!

BrianSidebotham commented 8 years ago

I get the following error now when trying to migrate the settings with the latest commits under windows:

Atom Version: 1.6.2 System: Microsoft Windows 7 Professional Thrown From: atom-beautify package, v0.29.1

Stack Trace

Uncaught TypeError: _.toPairs is not a function

At /C:/Users/Brian.sidebotham/.atom/packages/atom-beautify/src/beautify.coffee:463

TypeError: _.toPairs is not a function
    at atom-workspace.plugin.migrateSettings (file:///C:/Users/Brian.sidebotham/.atom/packages/atom-beautify/src/beautify.coffee:535:16)
    at CommandRegistry.module.exports.CommandRegistry.handleCommandEvent (C:\Users\Brian.sidebotham\AppData\Local\atom\app-1.6.2\resources\app.asar\src\command-registry.js:260:29)
    at C:\Users\Brian.sidebotham\AppData\Local\atom\app-1.6.2\resources\app.asar\src\command-registry.js:3:61
    at CommandPaletteView.module.exports.CommandPaletteView.confirmed (C:\Users\Brian.sidebotham\AppData\Local\atom\app-1.6.2\resources\app.asar\node_modules\command-palette\lib\command-palette-view.js:183:32)
    at CommandPaletteView.module.exports.SelectListView.confirmSelection (C:\Users\Brian.sidebotham\AppData\Local\atom\app-1.6.2\resources\app.asar\node_modules\atom-space-pen-views\lib\select-list-view.js:338:21)
    at space-pen-li.<anonymous> (C:\Users\Brian.sidebotham\AppData\Local\atom\app-1.6.2\resources\app.asar\node_modules\atom-space-pen-views\lib\select-list-view.js:139:19)
    at HTMLOListElement.jQuery.event.dispatch (C:\Users\Brian.sidebotham\AppData\Local\atom\app-1.6.2\resources\app.asar\node_modules\jquery\dist\jquery.js:4435:9)
    at HTMLOListElement.elemData.handle (C:\Users\Brian.sidebotham\AppData\Local\atom\app-1.6.2\resources\app.asar\node_modules\jquery\dist\jquery.js:4121:28)

Commands

     -0:30.3.0 atom-beautify:beautify-editor (atom-text-editor.editor.is-focused)
     -0:22.7.0 command-palette:toggle (atom-text-editor.editor.is-focused)
     -0:16.4.0 atom-beautify:migrate-settings (atom-text-editor.editor)

Config

{
  "core": {},
  "atom-beautify": {
    "_analyticsUserId": "brian.sidebotham",
    "c_configPath": "C:\\5135\\cpum-TRUNK\\uncrustify.cfg",
    "cpp_configPath": "C:\\5135\\cpum-TRUNK\\uncrustify.cfg",
    "general": {
      "_analyticsUserId": "6e7d8017-94f3-4e73-976f-456ff78ca4e9"
    }
  }
}

Installed Packages

# User
atom-beautify, v0.29.1
atom-typescript, v8.8.0
autocomplete-clang, v0.8.9
autocomplete-cmake, v0.3.1
build, v0.58.0
build-make, v0.8.0
language-cmake, v0.1.4
linter, v1.11.4
symbol-gen, v1.0.0

# Dev
No dev packages
Glavin001 commented 8 years ago

Publishing a new release soon. It should now look like: image

And it will need to be dismissed by the user, so it does not timeout and disappear before they can read it. :+1:

Glavin001 commented 8 years ago

Published patch to v0.29.2

@BrianSidebotham : This new release has updated Lodash dependency so that error should not occur this time. Let me know if you have any further issues. Thanks!

BrianSidebotham commented 8 years ago

@Glavin001 Updated and now works perfectly. Thanks very much for such an invaluable tool for me!

Glavin001 commented 8 years ago

Awesome to hear! :smiley:

derwaldgeist commented 8 years ago

Thanks for the patch, the new dialog definitely makes much more sense! I'm closing this issue now. And thanks again for this nice beautifier tool. It's just indispensable.

robjac commented 8 years ago

I'm still getting:

Please run Atom command 'Atom-Beautify: Migrate Settings'. You have unsupported options: _analyticsUserId

On beautifying HTML(Rails) contents in my app.

On Atom 1.7.2; updated this morning, hoping to fix the above warning; but it still persists for me.

Glavin001 commented 8 years ago

@robjac does running the Atom command Atom-Beautify: Migrate Settings not work to resolve this issue? What version of Atom-Beautify are you on? Make sure to update that as well.

robjac commented 8 years ago

Hey @Glavin001 ; I'll start by saying I'm big fan of Atom-Beautify.

I'm on Atom 1.7.2 , and Atom-Beautify 0.29.4

I had to re-run Atom-Beautify: Migrate Settings . Not sure why, but all good now! Thanks

Glavin001 commented 8 years ago

Excellent to hear it is working now for you 😃. Thanks for using Atom-Beautify!

For more information, this Pull Request is related to the new settings and why migrating the settings is required: https://github.com/Glavin001/atom-beautify/pull/864

Let me know if you need anything else. Thanks!