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

JS Chained methods never break regardless of "Break Chained Method" setting #836

Closed
ProustIdee opened this issue Feb 27, 2016 · 16 comments
Closed

Comments

@ProustIdee
Copy link

With Atom-Beautify 0.28.24.

Input code:

<html>
  <script>
    d3.csv("/data/test.csv", function (d) {
      svg.append('g')
      .selectAll('path');
    });
  </script>
</html>

Result of Beautifying:

<html>
  <script>
    d3.csv("/data/test.csv", function (d) {
      svg.append('g').selectAll('path');
    });
  </script>
</html>

It doesn't matter what I set "Javascript - Break chained methods" to. I'm using the JS Beautify engine.

I didn't have this problem until the last day or two. I assume Atom-Beautify was updated recently?

@ProustIdee ProustIdee changed the title Chained methods always break regardless of "Break Chained Method" setting JS Chained methods always break regardless of "Break Chained Method" setting Feb 27, 2016
@ProustIdee ProustIdee changed the title JS Chained methods always break regardless of "Break Chained Method" setting JS Chained methods never break regardless of "Break Chained Method" setting Feb 27, 2016
@Glavin001
Copy link
Owner

Please follow the Issue template provided, it is required not a recommendation. For your convience, here it is again: https://github.com/Glavin001/atom-beautify/blob/master/ISSUE_TEMPLATE.md
The most important part being the Gist containing your debugging information. That information would help me resolve many questions I will be asking without a tedious back and forth.

I await your debugging information. Thank you.


Possibly related issues: #831, #830
I believe this issue could be the same. There was a settings bug (incomplete feature) published in v0.28.23 and some users made changes in v0.28.23 and found that their settings did not work. This is because in v0.28.23 the settings groups do not work and in v0.28.24 I have reverted the groups (should not have been pushed, incomplete feature) so those settings that were set in the groups are ignored.

@ProustIdee
Copy link
Author

I uninstalled Atom-Beautify and reinstalled, I also deleted the Atom-beautify section from my config.cson file.

Here is the debug.md: https://gist.github.com/ProustIdee/d0eb29920a3addc00409

@Glavin001
Copy link
Owner

The option you used is:

    "js_break_chained_methods": true

Which is language namespace js.

While the language namespace for your code is html, as the grammar detected is HTML.

So Atom Beautify filtered out the options only for html namespace.

I do not see the break_chained_methods in the list of supported HTML language options: https://github.com/Glavin001/atom-beautify/blob/master/src/languages/html.coffee#L28
Feel free to submit a Pull Request, however using the .jsbeautifyrc file can allow any option.

I'd recommend setting break_chained_methods for html namespace in your .jsbeautifyrc file.

Something like:

{
  "html": { "break_chained_methods": true }
}

@Glavin001 Glavin001 added this to the v0.29.0 milestone Feb 27, 2016
@Glavin001 Glavin001 self-assigned this Feb 27, 2016
@ProustIdee
Copy link
Author

Thanks for the reply. Is there a reason this worked until recently?

The way that I understand it, I should have separate js files and call them from the html file, rather than writing script in the <script> </script> tags in the html file.

However, I didn't have any problems with this until recently, everything worked perfectly.

@Glavin001
Copy link
Owner

So you had a single HTML file with JavaScript content and using the js_break_chained_methods option everything worked before as expected and now it does not? Strange.
The only thing I could suggest is that maybe Pretty Diff (your currently selected beautifier) has been updated. /cc @prettydiff

@Glavin001 Glavin001 assigned prettydiff and unassigned Glavin001 Feb 27, 2016
@Glavin001
Copy link
Owner

Updates published to npm for dependencies, such as Pretty Diff or JS-Beautify, are not automatically applied to Atom Beautify in Atom, nor is Atom Beautify even shown as needing an update. However the dependency versions required for dependencies are very loose within Atom Beautify, such that by reinstalling Atom Beautify you can effectively update to the latest version of all the dependencies. For example, Pretty Diff: "prettydiff": "^1.16.17", at https://github.com/Glavin001/atom-beautify/blob/master/package.json#L97

So when you updated Atom Beautify, which has not happened in a while, you likely did update Pretty Diff and those changes made 13 days ago.

@prettydiff this may be an issue on your end then. Let me know if there's anything I can help with.

@prettydiff
Copy link
Collaborator

Looking at this now. When I run the sample code against the methodchain option used internally by Pretty Diff at http://prettydiff.com/ the output is as expected for the three supported values: chain, indent, none. It appears to correct on the Pretty Diff side.

I am examining this operation in Atom now.

@prettydiff
Copy link
Collaborator

I think I found the problem in Atom. You can reproduce the problem if Pretty Diff is the beautifier for both HTML and JavaScript. I think this problem is this following line where Pretty Diff is being passed a boolean for its methodchain option:

I am verifying now and also looking to see when/why this was changed on the Pretty Diff side.

@prettydiff
Copy link
Collaborator

Disregard the previous comment. I appear to be handling boolean values for that option correctly for backwards compatibility. That is not the problem.

The problem is that the language grammar is HTML. When I isolate the JavaScript code and change the language grammar to JavaScript everything work as expected. For some reason the option is not being passed through from HTML to JavaScript in Atom Beautify. I am looking to where this gets dropped.

@prettydiff
Copy link
Collaborator

I am not sure how to resolve this. I think the error is the settings screen for Atom Beautify (though I have not confirmed this and dont know how). You can set a value for a JavaScript option on that page, and once that option is set the value is cached for that given language grammar. HTML, a different language grammar with a different set of options, sets and applies its options independently. When the current language grammar is HTML you can change JavaScript options to no effect. When you change the language grammar to JavaScript the change in effect is manifest.

I have confirmed that the option setting works correctly if the language grammar is JavaScript. I have also confirmed that whatever setting was chosen for the given option is "locked in" once you change your grammar back to HTML. So long as the grammar is HTML the only thing that is broken is the JavaScript language option on the settings screen.

@prettydiff prettydiff assigned Glavin001 and unassigned prettydiff Mar 9, 2016
@Glavin001 Glavin001 removed this from the v0.29.0 milestone Mar 30, 2016
@timothyerwin
Copy link

I had the same problem...I had to change the file type in Atom to "JavaScript" as it was set to "Babel ES6 Javascript"

@prettydiff
Copy link
Collaborator

The solution proposed by @timothyerwin makes me happy. The crux of the problem is that this JavaScript option needs to be exposed for HTML grammars.

@Glavin001 Glavin001 removed their assignment Apr 20, 2017
@prettydiff
Copy link
Collaborator

This issue is likely resolved as the latest version of Pretty Diff recently landed in Atom Beautify. If I am in error please reopen this issue.

@stevenzeck
Copy link
Contributor

@prettydiff the updates have been published to apm yet, so you won't see changes yet. Should be soon though

@prettydiff
Copy link
Collaborator

@szeck87 I am aware. Closing all these issues might be a bit premature for the users, but I still feel good about cleaning house and clearing out some of the backlog. Excellent work on that pull request by the way!

@JeffML
Copy link

JeffML commented Mar 2, 2018

Disabling 'Break chained methods' in Javascript settings didn't work for me. I had to disable it in JSX settings.

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

No branches or pull requests

6 participants