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

Editor options of atom are ignored #894

Closed
3 tasks done
grma1 opened this issue Apr 5, 2016 · 13 comments · Fixed by #1598
Closed
3 tasks done

Editor options of atom are ignored #894

grma1 opened this issue Apr 5, 2016 · 13 comments · Fixed by #1598

Comments

@grma1
Copy link

grma1 commented Apr 5, 2016

Description

It seems, that atom-beautify is using it's own default options (since the better-settings change) and not atom's editor settings.

I'm not using a .jsbeautifyrc nor a .editorconfig

Expected Results

The beautified code should have looked like:

function Test() {
  var a,
    b,
    c;
}

Steps to Reproduce

  1. Add code to Atom editor
  2. Run command Atom Beautify: Beautify Editor
  3. This beautified code does not look right!

Debug

Here is a link to the debug.md Gist: https://gist.github.com/grma1/1ca0b62883e4294c333642e831762f48

How to create debug.md Gist

Note that this will include a copy of your code.
If your code is private, please create a different sample of code that reproduces the problem.

  1. In the Atom command-palette,
    search for and run the command Atom Beautify: Help Debug Editor.
    The debugging results will be copied to your clipboard.
  2. Create a new Gist at https://gist.github.com/
  3. Create a file in your new Gist called debug.md.
  4. Paste your debugging results from Atom beautify into debug.md file in your Gist.
  5. Add a link to your Gist in your new Issue.

Checklist

  • I have tried uninstalling and reinstalling Atom Beautify to ensure it installed properly
  • Searched for existing Atom Beautify Issues at https://github.com/Glavin001/atom-beautify/issues
    so I know this is not a duplicate issue
  • Generated debugging information and added link for debug.md Gist to this issue
@grma1 grma1 mentioned this issue Apr 5, 2016
3 tasks
@Glavin001
Copy link
Owner

I believe what you want is js.indent_size = 2 however your current configuration is js.indent_size = 4. See https://gist.github.com/grma1/1ca0b62883e4294c333642e831762f48#final-options

@Glavin001 Glavin001 added this to the v0.30.0 milestone Apr 5, 2016
@Glavin001 Glavin001 self-assigned this Apr 5, 2016
@grma1
Copy link
Author

grma1 commented Apr 5, 2016

Yes I want an indent of 2.
See https://gist.github.com/grma1/1ca0b62883e4294c333642e831762f48#beautification-options (Editor Options: Options from Atom Editor settings)
With previous versions of atom-beautify there was no need to configure the settings for each language in atom-beautify.

Maybe it has something to do with the "Migrate Settings" command I've triggered before.

I think atom-beautify should use atom's options as default value, which can be then be overwritten per language.

So I don't know whether it's a bug or a feature ;-)

@Glavin001
Copy link
Owner

Similar to how CSS applies rules, Atom-Beautify favours specificity over general/default configuration. For instance, the per-language Atom-Beautify package settings will override the global/default Editor Options assigned for each language.
However, because I do agree that the Editor Options should be taken into consideration, the default values of the Atom-Beautify package settings for each language are set to be the values from the Editor Options.
See https://github.com/Glavin001/atom-beautify/blob/master/src/languages/javascript.coffee#L3-L39
The default indent_size for JavaScript language should be properly extracted from those Editor Options.
I think it could be:

  1. you may need to re-install Atom-Beautify if you have recently made changes to Editor Options after installing Atom-Beautify package, since those default values are stored in src/options.json and built on postinstall. This is something that has changed with v0.29.x to improve performance by building the options during install once instead of every time Atom-Beautify activates.

Overall, I recommend that you set the Atom-Beautify package settings to your desired configuration. If you want to have a default, you can add a .jsbeautifyrc to your home directory using the key _default.

For example:

{
"_default": { "indent_size": 2 }
}

@iagoqo
Copy link

iagoqo commented Apr 13, 2016

I'm having the same issue. I tried reinstalling atom-beautify so it would load the default indent size from my editor options but it still set all the default indents to 4.

@Glavin001
Copy link
Owner

One idea would be to add a "Rebuild Options" command to Atom-Beautif that would run the build-options.js script.
I would be open to reviewing and merging a Pull Request for such a feature.

However, I will be unable to work on Atom-Beautify until May at the earliest, since I have my thesis and final exams to work on.

My recommendation is for everyone to change their settings in Atom-Beautify package settings instead.

@iagoqo
Copy link

iagoqo commented Apr 13, 2016

I did a find and replace for "default": 4 on src/options.json as a quick workaround, since changing the settings for all languages is a bit tedious. I had to reload atom for it to read the new defaults.

@seangates
Copy link

+1 for having atom-beautify honor my editor's configuration for indent_size for all languages.

@Glavin001
Copy link
Owner

The temporary workaround I recommend is to add a .jsbeautifyrc file in your home directory:

{
  "_default": { "indent_size": 2 }
}

This will take precedence over both Atom's Editor and Atom-Beautify's package default settings, while being applicable for all languages and also not being required per project.

This issue needs more discussion. Please provide your feedback.
The reason for this change in v0.29.x was to improve loading time of Atom-Beautify, which will occur on first save and building all of the options can be a time consuming task. With this change now it should load quickly on save along with loading the Settings View faster, too.
However, we now do not have the advantage of the dynamically loading the Atom's Editor settings, as shown here: https://github.com/Glavin001/atom-beautify/blob/master/src/languages/javascript.coffee#L3
Thus, we have a tradeoff between performance (improved by caching to src/options.json) versus flexibility (lost because the default options now ignore Atom's Editor settings).

An idea of the top of my head: update selected language options on load using a new Language attribute.
Take for example JavaScript Language: https://github.com/Glavin001/atom-beautify/blob/master/src/languages/javascript.coffee

Instead of the following:

# Get Atom defaults
scope = ['source.js']
tabLength = atom?.config.get('editor.tabLength', scope: scope) ? 4
softTabs = atom?.config.get('editor.softTabs', scope: scope) ? true
defaultIndentSize = (if softTabs then tabLength else 1)
defaultIndentChar = (if softTabs then " " else "\t")
defaultIndentWithTabs = not softTabs

module.exports = {

  name: "JavaScript"
  namespace: "js"

  # ...

  ###
  ###
  options:
    # JavaScript
    indent_size:
      type: 'integer'
      default: defaultIndentSize
      minimum: 0
      description: "Indentation size/length"

We can change it to:

module.exports = {

  name: "JavaScript"
  namespace: "js"
  # Add the language scope
  scope: "source.js"

  # Options that should look for default values from another option
  option_aliases:
    indent_size:
      path: 'editor.tabLength' # => atom.config.get('editor.tabLength', scope: this.scope)

  ###
  ###
  options:
    # JavaScript
    indent_size:
      type: 'integer'
      default: 4
      minimum: 0
      description: "Indentation size/length"

If this issue affects you, please provide your feedback! Thanks!

@seangates
Copy link

seangates commented Nov 7, 2016

When you say in your home directory, do you mean the home directory of your atom configs, or the home directory of your atom project? Or somewhere else?

UPDATE: The home directory is the home directory of your project.

@Glavin001
Copy link
Owner

Glavin001 commented Jan 3, 2017

I am working on this right now.

A couple changes:

  • Atom-Beautify's language options (such as indent_size and indent_char) do not need default values
  • postinstall script in package.json will be removed
    • The options.json will be pre-generated before publishing and therefore not cause any more issues upon installation with apm install atom-beautify

I have it working right now: https://gist.github.com/Glavin001/5b388d3836f5319d3e055de77a151ead
Still testing to make sure everything is OK.
As you can see in the Gist the Editor Options are:

{
    "_default": {
        "indent_size": 4,
        "indent_char": " ",
        "indent_with_tabs": false
    }
}

which are set under for the language-javascript / JavaScript Grammar and fallback with the settings found in Editor settings tab. In this test, I changed Tab Length.
With the default values of Atom Beautify's Package settings now being undefined the values from Atom's Editor settings are correctly passed through as we would expect!

@seangates
Copy link

@Glavin001 Thank you!!

@Glavin001
Copy link
Owner

Looks like it will not be as simple as I had hoped. Setting the config defaults to undefined or null causes Atom's Settings View to not show the option!

image

Here is a snippet of the example schema I am testing:

module.exports = {
  general:
    title: 'General'
    type: 'object'
    collapsed: true
    order: -1
    description: 'General options for Atom Beautify'
    properties:
      test1:
        type: "integer"
        default: null
      test2:
        type: "integer"
        default: undefined
      test3:
        type: "integer"
        default: 0
# ...
}

Settings View uses settings = atom.config.get(namespace) to get a list of options: https://github.com/atom/settings-view/blob/master/lib/settings-panel.coffee#L35

However, executing atom.config.get('atom-beautify.general') returns the following:

{
  "_analyticsUserId": "06c7bd24-f1e8-4120-b2b0-34f8e8d565e0",
  "analyticsUserId": "a8e901dd-5205-4caa-804e-933219818e59",
  "test3": 0,
  "analytics": true,
  "loggerLevel": "warn",
  "beautifyEntireFileOnSave": true,
  "muteUnsupportedLanguageErrors": false,
  "muteAllErrors": false
}

Notice test1 and test2 options are missing!

So while I can get this feature to work it is at the cost of breaking Atom-Beautify's package settings as shown in Settings View.

I think the best plan of attack is to fix Atom so it correctly adds the option to the schema instead of only showing a warning: https://github.com/atom/atom/blob/0e992326154d387611543f618ad5872dc4665586/src/config.coffee#L971
The applicable code is: https://github.com/atom/atom/blob/0e992326154d387611543f618ad5872dc4665586/src/config.coffee#L955-L972

  setRawDefault: (keyPath, value) ->
    setValueAtKeyPath(@defaultSettings, keyPath, value)
    @emitChangeEvent()

  setDefaults: (keyPath, defaults) ->
    if defaults? and isPlainObject(defaults)
      keys = splitKeyPath(keyPath)
      @transact =>
        for key, childValue of defaults
          continue unless defaults.hasOwnProperty(key)
          @setDefaults(keys.concat([key]).join('.'), childValue)
    else
      try
        defaults = @makeValueConformToSchema(keyPath, defaults)
        @setRawDefault(keyPath, defaults)
      catch e
        console.warn("'#{keyPath}' could not set the default. Attempted default: #{JSON.stringify(defaults)}; Schema: #{JSON.stringify(@getSchema(keyPath))}")
    return

Notice that within the catch it shows the warning and does not call setRawDefault. I think this is why the options test1 and test2 are missing, since they both cause the error.

The problem with editing Atom and/or Settings View is then users will be required to 1) wait for another Atom release and 2) must update their Atom such that they can use Atom-Beautify as they previously were. Without updating Atom, these changes to Atom-Beautify would appear to be breaking changes, as they will literally break the Settings View for certain Atom-Beautify 😞.

Glavin001 added a commit that referenced this issue Apr 15, 2017
Fixes #894. Default options are extracted from Atom Editor options
@Glavin001
Copy link
Owner

I've merged #1598 which should resolve these issues.

Published to v0.29.21.

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

Successfully merging a pull request may close this issue.

4 participants