Javascript Trailing Commas

[jonadeline]

Hi,

I'm using JSCS with airbnb preset and it recommends to use additional trailing commas but unfortunately atom-beautify removes them provoking JSCS error. Is there any way to fix this ?

Thanks in advance

[Glavin001]

The two JavaScript beautifiers that are currently supported are Pretty Diff and JS Beautify.

I do not believe that JS Beautify supports trailing commas: https://github.com/beautify-web/js-beautify/search?utf8=%E2%9C%93&q=trailing+comma
You could make an issue on their repository for that feature.

Likewise, I do not see this functionality for Pretty Diff either: https://github.com/prettydiff/prettydiff/search?utf8=%E2%9C%93&q=trailing+comma
@prettydiff could you comment on this? Would this be something you could add support for? Thank you in advance!

[prettydiff]

I am probably stripping trailing commas. Before ES5 trailing commas were a syntax error and even after they were no longer an error in JavaScript they are generally regarded as either nonsense or a bad practice. I could create a new option to forcefully inject a trailing comma in object literals. This would mean trailing commas are either always present or never present.

prettydiff/prettydiff#160

[Glavin001]

This has been addressed by Pretty Diff with #524 (comment)
For those interested in contributing with a Pull Request, you can very easily add the missing options by reading #524 (comment)

I am going to mark this issue as a duplicate of #524 now.

[Glavin001]

Published to v0.28.12 as JavaScript option end_with_comma.

[jonadeline]

@Glavin001,

The end_with_commas doesn't work in JSX files whereas it seems supported by Prettydiff.
Is it possible you add it for those kind of files as well ?

Thanks.

[Glavin001]

Should work.

Support is on JavaScript language: https://github.com/Glavin001/atom-beautify/blob/master/src/languages/javascript.coffee#L109-L113
And JSX language should pull in options including JavaScript: https://github.com/Glavin001/atom-beautify/blob/master/src/languages/jsx.coffee#L5

Please follow https://github.com/Glavin001/atom-beautify/blob/master/CONTRIBUTING.md#new-issues-bugs-questions-etc and share a Gist of your results of running command Atom Beautify: Help Debug Editor. Thanks.

[jonadeline]

Ok I think I find out why it didn't work. I was in Babel ES6 JavaScript file grammar and when I switch to Javascript it works.
Is that a normal behavior ?

https://gist.github.com/Bodhiz/0eab2e8cb0385e61ef9c

[Glavin001]

I was in Babel ES6 JavaScript file grammar and when I switch to Javascript it works.

This could make sense.

You had the following:

• grammar: Babel ES6 JavaScript
• extension: .jsx

Atom Beautify first determines the appropriate language to use and then finds the appropriate beautifiers for that language.
The language will be looked up based on the grammar first and then then the extension, and since Babel ES6 JavaScript has no language associated with that grammar it would find the language using the extension .jsx which is language JSX.

See applicable code:

• Getting languages given grammar and extension: https://github.com/Glavin001/atom-beautify/blob/master/src/beautifiers/index.coffee#L303-L308
• Getting languages given search criteria (note that grammar matches will be shown first): https://github.com/Glavin001/atom-beautify/blob/master/src/languages/index.coffee#L88-L89

The default beautifier for JSX language is Pretty Diff: https://github.com/Glavin001/atom-beautify/blob/master/docs/options.md#language-config---jsx---default-beautifier

When you switched the grammar to JavaScript there was a match for Atom Beautify language JavaScript (not language JSX).

The default beautifier for JavaScript is JS Beautify: https://github.com/Glavin001/atom-beautify/blob/master/docs/options.md#language-config---javascript---default-beautifier
This is what you indicated does work.

Looking at your debug.md I see:

indent_size=2, indent_char= , indent_with_tabs=false, space_before_conditional=false, end_with_comma=true, indent_size=2, indent_char= , indent_level=0, indent_with_tabs=false, preserve_newlines=true, max_preserve_newlines=10, space_in_paren=false, jslint_happy=false, space_after_anon_function=false, brace_style=collapse, break_chained_methods=false, keep_array_indentation=false, keep_function_indentation=false, eval_code=false, unescape_strings=false, wrap_line_length=0, end_with_newline=false, configPath=, configPath=, configPath=, configPath=, indent_size=2, indent_char= , selector_separator_newline=false, newline_between_rules=false, preserve_newlines=false, wrap_line_length=0, indent_comments=true, force_indentation=false, convert_quotes=none, align_assignments=false, no_lead_zero=false, configPath=, predefinedConfig=csscomb, configPath=, indent_size=2, indent_char= , emacs_path=, emacs_script_path=, indent_inner_html=false, indent_size=2, indent_char= , brace_style=collapse, indent_scripts=normal, wrap_line_length=250, wrap_attributes=auto, wrap_attributes_indent_size=2, preserve_newlines=true, max_preserve_newlines=10, unformatted=[a, sub, sup, b, i, u], end_with_newline=false, extra_liners=[head, body, /html], configPath=, configPath=, configPath=, perltidy_profile=, cs_fixer_path=, fixers=, level=, max_line_length=79, indent_size=4, ignore=[E24], indent_size=2, indent_char= , rustfmt_path=, indent_size=2, keywords=upper, identifiers=lower, configPath=, , indent_style=space, indent_size=2, end_of_line=lf, charset=utf-8, trim_trailing_whitespace=true, insert_final_newline=true, e4x=true, tab_width=2, indent_char= , , , , , , , , 
2015-12-04T16:49:34.633Z - verbose: [beautifiers/index.coffee] beautifier Pretty Diff
2015-12-04T16:49:34.633Z - verbose: [beautifiers/beautifier.coffee] prettydiff inchar= , insize=2, objsort=false, preserve=all, cssinsertlines=undefined, comments=indent, force=undefined, quoteConvert=undefined, vertical=none, wrap=0, space=false, noleadzero=undefined, endcomma=true, methodchain=true, source=class DtFiltersPage {

Note that in the above the option end_with_comma: true was successfully converted to endcomma=true by Atom Beautify, where endcomma is the expected option by Pretty Diff.

Looking at Pretty Diff's documentation I do see endcomma is the correct option: http://prettydiff.com/documentation.xhtml#options_definitions

So given all of this, it would suggest that Pretty Diff is not properly handling the endcomma option for JSX.
I would recommend first ensuring that Pretty Diff is up-to-date, by uninstalling and re-installing Atom Beautify which will trigger a fresh install of the latest Pretty Diff and other dependencies.
If that does not work, then I recommend asking @prettydiff for help.

Hope that helps!

[prettydiff]

I can confirm its not working at http://prettydiff.com/ so it definitely looks like a Pretty Diff defect. I will have to track down why this isn't working. Its working on my local.

[prettydiff]

Nevermind, I had the styleguide option set to crockford which explicitly turns this feature off. Once I reset the styleguide option I can toggle this option on and off correctly in production. Everything appears correct from the Pretty Diff side, but I am continuing to investigate.

[vviikk]

Anyone figured out how to add the trailing comma when beautify is triggered? I'm on a regular ES6 .js file. Not .jsx. Tried all 3 beautifiers (jsbeautufy, JSCS fixer & prettydiff). None worked. I have the trailing commas switched on.

Latest version of atom-beautify.

[prettydiff]

@piggyslasher Would you mind putting up a gist of a code sample that is giving you trouble.

I can reproduce the issue with this code:

var a = {john:"boy",bobbi:"girl"};

I will have to look into this further. With the endcomma option enabled this appears to function correctly at http://prettydiff.com/ and the option appears to be registered correctly in atom-beautify after looking at:

• https://github.com/Glavin001/atom-beautify/blob/master/src/languages/javascript.coffee
• https://github.com/Glavin001/atom-beautify/blob/9a203b86846e6ee2f461b63526f7b21649a8765c/src/beautifiers/prettydiff.coffee

[prettydiff]

Here is my debug log: https://gist.github.com/prettydiff/4f34ed2f343cda0a1de0

Everything looks correct on the Atom Beautify side of the house as the value is set to true and is correctly translated to the proper option name in prettydiff. This is not broken at http://prettydiff.com/?m=beautify

I am going to see if I can get output into the Chrome console that comes with Atom and see what is being input to the included prettydiff versus the output.

[prettydiff]

I found the defect on the Pretty Diff side. Atom Beautify is working correctly. The fix is to convert https://github.com/prettydiff/prettydiff/blob/master/lib/jspretty.js#L1901

to:

} else if ((x === "]" || x === "}") && options.endcomma === true && ltoke !== ",") {

My confusion earlier is that this defect is absent when object sorting is enabled. This is a Pretty Diff defect that will be fixed in the next release. I have already proven on my local that this fixes the problem in the atom-beautify package.

[vviikk]

You're a star mate. Will try this fix and get back to you.

[prettydiff]

@piggyslasher Just grab the latest version (v1.16.10). I published the fix on Friday.

[jonadeline]

Thx for the fix @prettydiff ;)
Just still get an issue when using this grammar (https://github.com/gandm/language-babel) with JSX file.
For example this snippet :

this.properties = {
      ajaxBody: {
        type: Object,
        value: {
          filters: [],
          searches: null,
          pageable: {},
        },
      },
      tagsList: {
        type: Array,
        value: [],
      },
};

becomes the following after beautification :

this.properties = {
      ajaxBody: {
        type: Object,
        value: {
          filters: [],
          searches: null,
          pageable: {}
        }
      },
      tagsList: {
        type: Array,
        value: []
      },
};

As you can see it removes some trailing commas. That doesn't occur when I'm using the Javascript grammar.

Have you got any idea where it comes from ?
Thanks in advance for your help.

[prettydiff]

Weird. I was able to reproduce this defect at first, but now I cannot. I cam continuing to look at this.

[jonadeline]

hi @prettydiff, did you find a way to handle that ? Thx.

[prettydiff]

Sorry, this fell off my radar. I am looking this morning and still cannot reproduce the defect. I looking at both the online tool and in Atom with Atom Beautify. At this point I am thinking there could be a conflict of options. I am going to need the generated Pretty Diff option object.

Could you go into the packages/atom-beautify/node_modules/prettydiff/lib directory and insert the line of code console.log(options) immediately prior to the tokenize function? On Windows and Mac the path is located in /Users/username/.atom

Then after you have saved that change you will need to restart Atom. When it opens back up you will need to open the Chrome console, which is cmd+opt+i on Mac or alt+ctrl+i on Windows.

Then after verifying that the option is JavaScript - End with comma is checked beautify the code. Copy the complete option object that is printed to the console and paste it in here.

Thanks.

[jonadeline]

@prettydiff, here it is :

Object {
accessibility: false
api: ""
braceline: false
bracepadding: false
braces: false
color: "white"
comments: "indent"
commline: false
compressedcss: false
conditional: false
content: false
context: ""
correct: false
crlf: false
cssinsertlines: false
csvchar: ","
diff: ""
diffcli: false
diffcomments: false
difflabel: "new"
diffspaceignore: false
diffview: "sidebyside"
dustjs: false
elseline: false
endcomma: true
force_attribute: false
force_indent: false
html: false
inchar: " "
inlevel: 0
insize: 2
jsscope: "none"
jsx: false
lang: "javascript"
langdefault: "text"
lineendcrlf: false
methodchain: "chain"
miniwrap: false
mode: "beautify"
neverflatten: false
nocaseindent: false
noleadzero: false
objsort: "none"
preserve: true
quote: false
quoteConvert: "none"
selectorlist: false
semicolon: false
source: "class dtSearchEngine {↵↵ beforeRegister() {↵ this.properties = {↵ engineCollapsed: {↵ type: Boolean,↵ value: true,↵ },↵ regions: {↵ type: Array,↵ value: [],↵ },↵ departements: {↵ type: Array,↵ value: [],↵ },↵ countries: {↵ type: Array,↵ value: [],↵ },↵ dialogElement: {↵ type: Object,↵ value: { opened: false },↵ },↵ tagsList: {↵ type: Object,↵ value: { countries:[{ id: 'FR', value: 'France' }], departements:[] },↵ },↵ hasDepartementTags: {↵ type: Boolean,↵ value: false,↵ },↵ isSelectAll: {↵ type: Boolean,↵ value: false,↵ },↵ sortType: {↵ type: String,↵ value: 'region',↵ observer: '_sortTypeChanged',↵ },↵ dialogClosed: {↵ type: Boolean,↵ value: true,↵ },↵ };↵↵ this.observers = [↵ '_departementsTagListChanged(tagsList.departements.*)',↵ '_countriesTagListChanged(tagsList.countries.*)',↵ ];↵↵ this.isNotEmptyContracts = totalElements => totalElements !== 0;↵ this.computeContractUrl = contract => `/api/contracts/${contract.id}`;↵ this.computeContractsUrl = this._computeContractsUrl;↵ this.numberOfContractsDisplayed = this._numberOfContractsDisplayed;↵ }↵↵ get behaviors() {↵ return [↵ DtComponentsBehavior,↵ DtToolsBehavior,↵ ];↵ }↵↵ _toggleEngineCollapse() {↵ this.$.collapse.toggle();↵ if (this.$.collapse.opened) {↵ this.engineCollapsed = false;↵ } else {↵ this.engineCollapsed = true;↵ }↵ }↵↵ _isSearchInProgress(searchString) {↵ if (searchString.length > 0) {↵ return true;↵ } else {↵ return false;↵ }↵ }↵↵ _handleRegionsResponse() {↵ this.regions = this.$.regionsRessource.lastResponse;↵ /* build also an array of all departements for departements sorted view */↵ for (let region of this.regions) {↵ for (let departement of region.departements) {↵ departement.regionId = region.id;↵ this.push('departements', departement);↵ }↵ }↵ }↵↵ _toggleDialog(event) {↵ let dialogId = Polymer.dom(event).localTarget.dataset.dialogId;↵ this.$$(`#${dialogId}`).toggle();↵ }↵↵ _computeCollapseId(id) {↵ return `collapseDepartement${id}`;↵ }↵↵ _computeCheckboxRegionId(id) {↵ return `checkboxRegion${id}`;↵ }↵↵ _computeCheckboxDepartementId(id) {↵ return `checkboxDepartement${id}`;↵ }↵↵ _computeCheckboxDepartementValue(id, name) {↵ return `${id} - ${name}`;↵ }↵↵ _toggleDepartement(event) {↵ let iconElement = Polymer.dom(event).localTarget;↵ let regionId = iconElement.dataset.regionId;↵ let collapseElement = this.$$(`#collapseDepartement${regionId}`);↵ collapseElement.toggle();↵ this._resizeDialog();↵ if (collapseElement.opened) {↵ iconElement.icon = 'remove';↵ } else {↵ iconElement.icon = 'add';↵ }↵ }↵↵ _handleRegionCheck(event) {↵ let checkboxElement = Polymer.dom(event).localTarget;↵ let region = this._getRegion(checkboxElement.dataset.regionId);↵ this._setRegionCheckedState(region.id, checkboxElement.checked);↵ this._handleRelatedDepartements(region.id, checkboxElement.checked);↵ this._setSelectAll();↵ }↵↵ _handleDepartementCheck(event) {↵ let checkboxElement = Polymer.dom(event).localTarget;↵ let region = this._getRegion(checkboxElement.dataset.regionId);↵ let departement = this._getDepartement(checkboxElement.dataset.departementId);↵ this._setDepartementCheckedState(region.id, departement.id, checkboxElement.checked);↵ this._handleRelatedRegion(region.id, checkboxElement.checked);↵ this._setSelectAll();↵ }↵↵ _handleCountryCheck(event) {↵ let checkboxElement = Polymer.dom(event).localTarget;↵ let country = this._getCountry(checkboxElement.dataset.countryCode);↵ this._setCountryCheckedState(country, checkboxElement.checked);↵ this._setSelectAll();↵ }↵↵ _getRegion(regionId) {↵ for (let region of this.regions) {↵ if (parseInt(region.id) === parseInt(regionId)) {↵ return region;↵ }↵ }↵↵ return null;↵ }↵↵ _getDepartement(departementId) {↵ for (let region of this.regions) {↵ for (let departement of region.departements) {↵ if (departement.id === departementId) {↵ return departement;↵ }↵ }↵ }↵↵ return null;↵ }↵↵ _getCountry(countryCode) {↵ for (let country of this.countries) {↵ if (country.code === countryCode) {↵ return country;↵ }↵ }↵↵ return null;↵ }↵↵ _getDepartements(regionId) {↵ let region = this._getRegion(regionId);↵ if (region !== null) {↵ return region.departements;↵ } else {↵ return null;↵ }↵ }↵↵ _handleRelatedDepartements(regionId, checkedState) {↵ let departements = this._getDepartements(regionId);↵ let region = this._getRegion(regionId);↵ for (let departement of departements) {↵ this._setDepartementCheckedSta…') {↵ return this.isSelectAllDepartements;↵ } else if (this.dialogElement.id === 'countriesDialog') {↵ return this.isSelectAllCountries;↵ }↵ }↵↵ _removeTag(id) {↵ let tagType = this._getTagType();↵ for (let tag of this.tagsList[tagType]) {↵ if (tag.id === id) {↵ let index = this.tagsList[tagType].indexOf(tag);↵ this.splice(`tagsList.${tagType}`, index, 1);↵ }↵ }↵ }↵↵ _getTagType() {↵ if (this.dialogElement.id === 'regionsDialog') {↵ return 'departements';↵ } else if (this.dialogElement.id === 'countriesDialog') {↵ return 'countries';↵ }↵ }↵↵ _handleTag(id, name, relatedId, checkedState) {↵ if (!this._getSelectAll() && checkedState) {↵ this._addTag(id, name, relatedId);↵ } else if (!checkedState) {↵ this._removeTag(id);↵ }↵ }↵↵ _uncheckDepartement(event) {↵ let paperTagElement = Polymer.dom(event).localTarget;↵ let departement = this._getDepartement(paperTagElement.dataset.departementId);↵ let region = this._getRegion(paperTagElement.dataset.regionId);↵ this._setDepartementCheckedState(region.id, departement.id, false);↵ this._handleRelatedRegion(paperTagElement.dataset.regionId, false);↵ }↵↵ _uncheckCountry(event) {↵ let paperTagElement = Polymer.dom(event).localTarget;↵ let country = this._getCountry(paperTagElement.dataset.countryCode);↵ this._setCountryCheckedState(country, false);↵ }↵↵ _departementsTagListChanged() {↵ if (this.tagsList.departements.length === 0) {↵ this.hasDepartementTags = false;↵ } else {↵ this.hasDepartementTags = true;↵ }↵ }↵↵ _countriesTagListChanged() {↵ if (this.tagsList.countries.length === 0) {↵ this.hasCountriesTags = false;↵ } else {↵ this.hasCountriesTags = true;↵ }↵ }↵↵ _computeFilter(string) {↵ if (!string) {↵ // set filter to null to disable filtering↵ return null;↵ } else {↵ // return a filter function for the current search string↵ string = string.toLowerCase();↵ return location => {↵ var location = location.name.toLowerCase();↵ return (location.indexOf(string) != -1);↵ };↵ }↵ }↵↵ _clearInput(event) {↵ this.searchString = '';↵ }↵↵ _sortTypeChanged(newValue) {↵ this._clearInput();↵ if (newValue === 'departement' && this.departements.length === 0) {↵ for (let region of this.regions) {↵ for (let departement of region.departements) {↵ departement.regionId = region.id;↵ this.push('departements', departement);↵ }↵ }↵ }↵ }↵↵ _sortAlphabetically(a, b) {↵ if (a.name < b.name) {↵ return -1;↵ }↵↵ if (a.name > b.name) {↵ return 1;↵ }↵↵ return 0;↵ }↵↵ _sortDepartements(a, b) {↵ if (parseInt(a.id) < parseInt(b.id)) {↵ return -1;↵ }↵↵ if (parseInt(a.id) > parseInt(b.id)) {↵ return 1;↵ }↵↵ return 0;↵ }↵↵ _computeDialogSearchLabel(sortType) {↵ if (sortType === 'region') {↵ return 'Rechercher une région...';↵ } else {↵ return 'Rechercher un département...';↵ }↵ }↵↵ _setSortType(event) {↵ let paperGroupElement = Polymer.dom(event).localTarget;↵ this.sortType = paperGroupElement.selected;↵ }↵↵ /* Fired on user interaction only */↵ _selectAllChanged(event) {↵ let checkboxElement = Polymer.dom(event).localTarget;↵↵ if (this.dialogElement.id === 'regionsDialog' && this.regions !== undefined) {↵ for (let region of this.regions) {↵ this._setRegionCheckedState(region.id, checkboxElement.checked);↵ this._handleRelatedDepartements(region.id, checkboxElement.checked);↵ }↵ } else if (this.dialogElement.id === 'countriesDialog' && this.countries !== undefined) {↵ for (let country of this.countries) {↵ this._setCountryCheckedState(country, checkboxElement.checked);↵ }↵ }↵↵ this._resetTags();↵ }↵↵ _handleDialogClosing(event) {↵ this.notifyPath('dialogElement.opened', this.dialogElement.opened);↵ }↵↵ _handleDialogOpening(event) {↵ this.dialogElement = Polymer.dom(event).localTarget;↵ this.notifyPath('dialogElement.opened', this.dialogElement.opened);↵ }↵↵ _resetCheckedState() {↵ for (let i in this.regions) {↵ this.set(`regions.${i}.checked`, false);↵ for (let j in this.regions[i].departments) {↵ this.set(`regions.${i}.departements.${j}.checked`, false);↵ }↵ }↵↵ for (let i in this.departements) {↵ this.set(`departements.${i}.checked`, false);↵ }↵ }↵↵ _resetTags() {↵ let tagType = this._getTagType();↵ this.splice(`tagsList.${tagType}`, 0, this.tagsList[tagType].length);↵ }↵↵ _isDisplayRegions() {↵ if (!this.hasCountriesTags) {↵ return true;↵ } else {↵ for (let country of this.tagsList.countries) {↵ if (country.id === 'FR') {↵ return true;↵ }↵ }↵ }↵↵ return false;↵ }↵↵ _resizeDialog() {↵ this.dialogElement.notifyResize();↵ }↵↵ _fireSearch() {↵ }↵}↵↵Polymer(dtSearchEngine);↵ "
sourcelabel: "base"
space: false
spaceclose: false
style: "indent"
styleguide: ""
tagmerge: false
tagsort: false
ternaryline: false
textpreserve: false
titanium: false
topcoms: false
varword: "none"
vertical: "none"
wrap: 0
}

[prettydiff]

Excellent, I will examine this further later today.

[prettydiff]

@Glavin001 I am noticing when this option is checked and I beautify the code snippet in #534 (comment) the result is a toggling effect. Beautify the code sample several times and you can see commas flicker in and out. I am not sure that is causing that.

@Bodhiz I am still having trouble reproducing this the problem of only partial trailing commas.

[jonadeline]

Did you try with this grammar : https://github.com/gandm/language-babel ?

[prettydiff]

I installed that package, which created the grammar Babel ES6 JavaScript. When I attempted to beautify the code with that grammar I get an error that the grammar is not currently supported.

We need to extend Atom Beautify to support that grammar name and any unique file extensions it supports. Would you mind opening a new issue for this so that we can track the work appropriately?

[jonadeline]

You get that error directly in Atom ? Didn't get anything on my side.
Ok I'm gonna open a new issue for that. Thanks for your help.

[munjalpatel]

Unfortunately, this adds a trailing comma in an import statement as well:

import {validateToken, createSession, destroySession,} from '../../../actions/auth';

[prettydiff]

@munjalpatel Good catch. I opened a defect: prettydiff/prettydiff#308

[prettydiff]

@munjalpatel I looked into this matter. Pretty Diff will not conditionally retain trailing commas. Pretty Diff has an option called endcomma http://prettydiff.com/documentation.xhtml#endcomma

If this option is provide a value of false all trailing commas are removed. If true trailing commas are forcefully inserted. These changes occur without regard to whether a trailing comma is already present in the code sample.

I tested your code sample at http://prettydiff.com/?m=beautify with and without endcomma and it appears to be operating correctly.