{"action":"created","issue":{"url":"https://api.github.com/repos/CindyJS/CindyJS/issues/556","repository_url":"https://api.github.com/repos/CindyJS/CindyJS","labels_url":"https://api.github.com/repos/CindyJS/CindyJS/issues/556/labels{/name}","comments_url":"https://api.github.com/repos/CindyJS/CindyJS/issues/556/comments","events_url":"https://api.github.com/repos/CindyJS/CindyJS/issues/556/events","html_url":"https://github.com/CindyJS/CindyJS/pull/556","id":198006616,"number":556,"title":"Fix version of js-beautify","user":{"login":"gagern","id":190759,"avatar_url":"https://avatars.githubusercontent.com/u/190759?v=3","gravatar_id":"","url":"https://api.github.com/users/gagern","html_url":"https://github.com/gagern","followers_url":"https://api.github.com/users/gagern/followers","following_url":"https://api.github.com/users/gagern/following{/other_user}","gists_url":"https://api.github.com/users/gagern/gists{/gist_id}","starred_url":"https://api.github.com/users/gagern/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gagern/subscriptions","organizations_url":"https://api.github.com/users/gagern/orgs","repos_url":"https://api.github.com/users/gagern/repos","events_url":"https://api.github.com/users/gagern/events{/privacy}","received_events_url":"https://api.github.com/users/gagern/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":{"login":"strobelm","id":14185442,"avatar_url":"https://avatars.githubusercontent.com/u/14185442?v=3","gravatar_id":"","url":"https://api.github.com/users/strobelm","html_url":"https://github.com/strobelm","followers_url":"https://api.github.com/users/strobelm/followers","following_url":"https://api.github.com/users/strobelm/following{/other_user}","gists_url":"https://api.github.com/users/strobelm/gists{/gist_id}","starred_url":"https://api.github.com/users/strobelm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/strobelm/subscriptions","organizations_url":"https://api.github.com/users/strobelm/orgs","repos_url":"https://api.github.com/users/strobelm/repos","events_url":"https://api.github.com/users/strobelm/events{/privacy}","received_events_url":"https://api.github.com/users/strobelm/received_events","type":"User","site_admin":false},"assignees":[{"login":"strobelm","id":14185442,"avatar_url":"https://avatars.githubusercontent.com/u/14185442?v=3","gravatar_id":"","url":"https://api.github.com/users/strobelm","html_url":"https://github.com/strobelm","followers_url":"https://api.github.com/users/strobelm/followers","following_url":"https://api.github.com/users/strobelm/following{/other_user}","gists_url":"https://api.github.com/users/strobelm/gists{/gist_id}","starred_url":"https://api.github.com/users/strobelm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/strobelm/subscriptions","organizations_url":"https://api.github.com/users/strobelm/orgs","repos_url":"https://api.github.com/users/strobelm/repos","events_url":"https://api.github.com/users/strobelm/events{/privacy}","received_events_url":"https://api.github.com/users/strobelm/received_events","type":"User","site_admin":false},{"login":"montaga","id":15124152,"avatar_url":"https://avatars.githubusercontent.com/u/15124152?v=3","gravatar_id":"","url":"https://api.github.com/users/montaga","html_url":"https://github.com/montaga","followers_url":"https://api.github.com/users/montaga/followers","following_url":"https://api.github.com/users/montaga/following{/other_user}","gists_url":"https://api.github.com/users/montaga/gists{/gist_id}","starred_url":"https://api.github.com/users/montaga/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/montaga/subscriptions","organizations_url":"https://api.github.com/users/montaga/orgs","repos_url":"https://api.github.com/users/montaga/repos","events_url":"https://api.github.com/users/montaga/events{/privacy}","received_events_url":"https://api.github.com/users/montaga/received_events","type":"User","site_admin":false}],"milestone":null,"comments":2,"created_at":"2016-12-29T13:23:02Z","updated_at":"2016-12-29T17:53:00Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/CindyJS/CindyJS/pulls/556","html_url":"https://github.com/CindyJS/CindyJS/pull/556","diff_url":"https://github.com/CindyJS/CindyJS/pull/556.diff","patch_url":"https://github.com/CindyJS/CindyJS/pull/556.patch"},"body":"Different versions of js-beautify will format our codebase in different ways, even for patch-level version increments.  So I suggest we fix the version the hard way, with the intention of bumping the version every now and then in a conscious update, containing all related code formatting changes in that same commit.\r\n\r\nThis commit does *not* use the latest version at the time of the commit. That would be 1.6.7 which improves [one line of code](https://github.com/CindyJS/CindyJS/blob/8422099f65445e3f67a12ece5c4e22638ca464d0/plugins/cindygl/src/js/CodeBuilder.js#L296) thanks to a fix for https://github.com/beautify-web/js-beautify/issues/1079, but in [other](https://github.com/CindyJS/CindyJS/blob/v0.8.2/src/js/libcs/OpDrawing.js#L325-L329) [places](https://github.com/CindyJS/CindyJS/blob/v0.8.2/src/js/Events.js#L496) it suffers from https://github.com/beautify-web/js-beautify/issues/1090 and https://github.com/beautify-web/js-beautify/issues/1085#issuecomment-269628467 even though I don't know why earlier versions don't affect us there, too.\r\n\r\nI'd like to get this one here in soon, and then hope for the two issues mentioned above to get addressed. And then perhaps @montaga can have a look at bumping to something more recent, as most of the remaining differences are located in the CindyGL codebase."},"comment":{"url":"https://api.github.com/repos/CindyJS/CindyJS/issues/comments/269667149","html_url":"https://github.com/CindyJS/CindyJS/pull/556#issuecomment-269667149","issue_url":"https://api.github.com/repos/CindyJS/CindyJS/issues/556","id":269667149,"user":{"login":"gagern","id":190759,"avatar_url":"https://avatars.githubusercontent.com/u/190759?v=3","gravatar_id":"","url":"https://api.github.com/users/gagern","html_url":"https://github.com/gagern","followers_url":"https://api.github.com/users/gagern/followers","following_url":"https://api.github.com/users/gagern/following{/other_user}","gists_url":"https://api.github.com/users/gagern/gists{/gist_id}","starred_url":"https://api.github.com/users/gagern/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/gagern/subscriptions","organizations_url":"https://api.github.com/users/gagern/orgs","repos_url":"https://api.github.com/users/gagern/repos","events_url":"https://api.github.com/users/gagern/events{/privacy}","received_events_url":"https://api.github.com/users/gagern/received_events","type":"User","site_admin":false},"created_at":"2016-12-29T17:53:00Z","updated_at":"2016-12-29T17:53:00Z","body":"Having to stay current with some external library or API is a pain. But I don't see much motivation staying current with js-beautify: as long as it provides a consistent and sane coding style for our project, I think we can stick to one version for considerable time, updating only when we find that some idiom we like to use gets handled poorly by the current version but correctly by the next one. And even then, making all required changes is a question of automatically beautifying everything in the code base, and having a look to see that it doesn't add more confusion than it removes. I'd rather have such a commit every half a year or so, than having spurious indentation changes as unintended consequences of an unrelated commit.\r\n\r\nI know that the amount of curly braces is somewhat controversial. I've worked with projects that require curly braces for all control constructs that allow them, and although I can see the merit in how they enforce consistent coding, I think several rows of closing braces make the code considerably longer without adding any value. Consider for example this snippet from an older version of the conic drawing branch:\r\n\r\n```js\r\nsol = solveRealQuadratic(k00, -2 * k10, k20);\r\nif (sol)\r\n    for (i = 0; i < 2; ++i)\r\n        if (sol[i] >= 0 && sol[i] <= csw)\r\n            boundary.push(\r\n                mkp(sol[i], -0.5 * (c11 * sol[i] + c01) / c02));\r\n```\r\n\r\nIf I had to write that using braces, that would add three more rows of closing braces and in my opinion add zero value. The same holds for the various sanity checks we use for type checking, overflow detection and the likes. Things like `if (arg0.ctype !== \"number\") return nada;` or `if (!isFinite(x)) return;` are easily written as a single line and in that fashion don't distract from the main flow of control. Having to use three lines for each of these takes up too much space, in my opinion.\r\n\r\nI know that this view is contrary to many style guides, so I'd be willing to abide by a decision to the contrary within the team. But I would want such a decision to at least take these use cases into consideration, and not just blindly follow some style guide. And I'd definitely not want to adapt such a style change just to work around some bugs in our beautifier. I know KaTeX uses [eslint](http://eslint.org/) to style-check their sources, including required braces. I think it offers some more options than js-beautify. I have no experience whether its `--fix` mode is equivalent to the automated beautification js-beautify performs, but I guess it might. So if we are unhappy with js-beautify in some way, we might want to have a closer look at that.\r\n\r\nThe other changes I see between version 1.6.4 and 1.6.7 affects indentation of various CindyGL functions. I haven't even started to work out why these changed, but as the new indentation looks better to me, I guess it might be intentional. Nevertheless, if we don't fix the version but allow both 1.6.4 and 1.6.7, then different developers beautifying the same code in turns will switch between these versions, swamping the git history with noise. Plus I have no idea what kind of intentional or unintentional style changes future versions of js-beautify may bring, so I'd not assume that adding braces and fixing some indentation would guard us against those."}}
{"actor":{"display_login":"gagern"}}