• Formatting for JS/SSJS files

    From Michael J. Ryan@1:103/705 to GitLab issue in main/sbbs on Fri Apr 2 13:51:51 2021
    open https://gitlab.synchro.net/main/sbbs/-/issues/246

    Would be nice to be able to use a built-in or external formatting tool for the .js and .ssjs files.I would propose that we use prettier with the following settings for this.```yamltabWidth: 4useTabs: truesingleQuote: truetrailingComma: "es5"```The main issue is the use of "for each" is not understood, as it's deprecated and didn't make it into the language spec.https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/For-each-in_loops_are_deprecatedAs an intermediate step, with polyfills updated, could use. `Object.keys`, `Object.entries`, `Object.values` with `Array.prototype.forEach` iteration. Which isn't as efficient as a regular for-loop, but would allow for consistent linting and formatting to be added (as well as autoformatting in supportive editors).
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Deucе@1:103/705 to GitLab note in main/sbbs on Sat Apr 3 00:36:36 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1887

    Tab width shouldn't matter as long as you use spaces for alignment. If prettier can't use spaces for alignment, I suggest using a tool that can.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Michael J. Ryan@1:103/705 to GitLab note in main/sbbs on Mon Apr 5 19:33:03 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1910

    @Deuce prettier can absolutely use spaces instead of tabs... there's a mish-mash of tabs/spaces in the code, and my understanding per @rswindell was that it *should* be tabs with tabstops at 4... it's absolutely configurable. Would just remove the `useTabs` line above.@echicken should probably also chime in... Most open-source JS projects for the past several years uses two spaces (not tabs).Other tool options include `standard` and `eslint` ... Standard is very opinionated, eslint is very flexible, and prettier is somewhere in the middle.- https://prettier.io/ (10,659,405/week)- https://eslint.org/ (13,075,668/week) - https://www.npmjs.com/package/eslint-config-airbnb (2,045,349/week - popular eslint preset)- https://standardjs.com/ (220,686/week)
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Michael J. Ryan@1:103/705 to GitLab note in main/sbbs on Mon Apr 5 19:33:51 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1911

    Benefits to using eslint, is that it can also do linting with some (re)formatting support.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From echicken@1:103/705 to GitLab note in main/sbbs on Mon Apr 5 20:29:03 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1912

    My preference is tabs for identation and spaces for alignment. I follow a styleguide of my own which is a modified version of StandardJS. I don't use a linter or formatting tool for Synchronet work, since as things stand that can cause more hassle than it avoids. Historically, I have been all over the map on all of these points.I don't have strong feelings about this issue, but if there's consensus that some kind of standardization should happen, I'll dig deep and find some opinions.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Michael J. Ryan@1:103/705 to GitLab note in main/sbbs on Mon Apr 5 21:20:31 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1913

    @echicken I'd just like to contribute more, and have formatting setup via my editor to just format on save, can use a config that's agreed on... just find it slightly irritating to see pushback on whitespace changes when doing a merge request. I'd like to see a solution that's just a workable flow.I prefer two spaces, single-quotes, es5 commas at line end, and the rest of prettier, I'm pretty happy with in general.Something with a single pass to update everything, circle back and adjust the `for each` use, and reformatting on those files... then it's something that can be run as a regular change. I'm not really hard-nosed on the settings so much as something where it just formats "properly" for the project and consistently, which things aren't.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Rob Swindell@1:103/705 to GitLab note in main/sbbs on Mon Apr 5 22:49:08 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1914

    It's best to try to use an editor which doesn't reformat existing code, even if it's just white-space changes.There is no consistent style currently, so the general guidance is to use the existing style of the file you're editing. This is expectation of most open source projects. Nobody wants to have style wars or review diffs that contain a bunch of unnecessary/style changes in addition to real fixes or features.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Michael J. Ryan@1:103/705 to GitLab note in main/sbbs on Tue Apr 6 10:22:41 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1915

    @rswindell which is why I'd like to see the use of a formatter taken up... then it *CAN* be consistent. Even in the exec/load/arrays.js if you look at my original PR, there are some portions with tabs, others with spaces... in the same file. I'm not wanting to start a style war, just have some kind of consistency so that it can be, consistent. The actual settings, or how it's configured, I'm much less dogmatic about.Quite a few open-source projects (at least JS based ones) do use a linter and/or formatter. Some other languages even have one for canonical styling in the box.There's also https://editorconfig.org/ that many editors support or have plugins for.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Rob Swindell@1:103/705 to GitLab note in main/sbbs on Tue Apr 6 12:36:58 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1916

    Yup, see https://gitlab.synchro.net/main/sbbs/-/blob/master/src/.editorconfig (for the C/C++ source) but it hasn't really helped since not all contributors use editors that support it. It's mainly Deuce and I and he has been using 8 space tab stops for a few years now.It sounds like you're saying that you want to reformat all the JS code in the Synchronet git repo and you want this to happen so that you can contribute more. I don't see how the two are related. We've had several contributors over several years without having a consistent style or any kind of auto-reformatting.I'm not opposed to enforcing a consistent style, but am in no hurry to have the debate/discussion about what that style should be or how its enforced. <yawn>That said, Deuce and I are discussion a beautification of (most of) the C/C++ source in the Synchronet repo using uncrustify and then having a defined style to (hopefully) conform to going forward.Additionally, when bringing in 3rd party code, it's usually best practice to keep that code in the original style as much as possible so that diffing/merging from any upstream changes remains fairly painless. So there should be the allowance for exceptions to any style rule.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Deucе@1:103/705 to GitLab note in main/sbbs on Tue Apr 6 15:02:29 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1917

    The maintainer gets to pick the formatting for all files in their project. The contents of exec do not constitute a single project, and all the JS/SSJS content in git is /certainly/ not a single project.For any new files you authour, feel free to use any formatting you please. When modifying existing files, make an attempt to fit in with the code around the change. If a contribution is not accepted due to style issues, and you think the maintainer is being unreasonable (and you made an effort to be consistent), talk to @rswindell and he'll do whatever he thinks is appropriate. It is not unreasonable for a maintainer to expect the code he maintains to continue to look the same as it did last time he looked at it. When someone is volunteering their time to maintain code, applying arbitrary style changes to the code they are maintaining is unreasonable.Reformatting new code before accepting it is, in my opinion, much worse than a maintainer expecting his style to be retained. It is much more likely to alienate the authour and prevent him from becoming a maintainer. A situation where every contributor reformats every file they touch is even worse and makes the commit history border on useless.If you and some other maintainers want to get together and share a style for the code you maintain, go ahead... convert your files and do a single merge request for them together. I would only ask that you put a header in the files indicating what is expected from future contributors (ie: "please run through prettier with XXX settings before submitting a merge request"). When it's your code, you set the rules.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Nigel Reed@1:103/705 to GitLab note in main/sbbs on Tue Apr 6 15:45:30 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1918

    If I'm editing a file for some reason, I'll use the format it's written in. If I'm writing my own, which is rare, I'll usually use something else as a template and then use that. Historically, I've been a PERL scripter and have always used a 2 space indentation, the same with bash scripts. It leaves enough indentation to help line up sections without having stupid amounts of blank space for multi-indented sections. That just makes it hard to read unless you have a stupid wide terminal setting.I prefer readability.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Michael J. Ryan@1:103/705 to GitLab note in main/sbbs on Thu Apr 8 11:26:35 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1920

    @Deuce is the maintainer of every JS file identified in the file/project, or the projects identified in any meaningful way?
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Rob Swindell@1:103/705 to GitLab note in main/sbbs on Thu Apr 8 12:08:18 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1921

    There are several file/projects in the Synchronet git repo.The list of maintainers of JS files in the Synchronet git repo includes @cyan, @Deuce, @echicken, @mcmlxxix, @nightfox, and myself, at least.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From echicken@1:103/705 to GitLab note in main/sbbs on Thu Apr 8 13:28:32 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1922

    For stuff in exec/ and exec/load/, there isn't a consistent way to determine maintainership and project-membership at a glance. You'd either have to look at the commit log or ask (probably on IRC). Anything else would require a reorganizing/renaming/commenting/reformatting blitz.I have limited time for BBS stuff right now, so spending it on tooling and meta-work wouldn't be my first choice. Beyond that I'm not opposed to adopting a consistent style as long as it means next to zero extra work for me and I don't hate the new format. I'm sure we'll reach a consensus any day now.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Michael J. Ryan@1:103/705 to GitLab note in main/sbbs on Thu Apr 8 14:25:22 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1923

    @echicken I think for the most part, it would mean a prettier config, and an npx command to run (would require node be installed).There are other options etc. Would just like to have something consistent, and if a file is modified, update/format with that PR without issue, or reformat everything so whitespace/formatting no long a future issue `for each` not withstanding.
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Deucе@1:103/705 to GitLab note in main/sbbs on Thu Apr 8 17:33:20 2021
    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1924

    Changing whitespace in a PR when making a functional change is bad, and I hope we never encourage that. Reformatting everything would be highly disruptive, and there currently is no consensus between maintainers of what "right" would look like.For myself, I don't have node installed on my main development system and am not especially interested in dealing with npm there... which is not to say I'm interested in having all my files modified by some other tool either.I honestly think the best thing to do now that we have gitlab would be to hack together something like npm for Synchronet to manage a users mods directory (or more likely a new spm load path) and solve the problem of tracking maintainership and exec clutter. Just an spm load path thing of course wouldn't be enough because we also want at least xtrn and web stuff to be installable in the same basic manner. Such a thing would exec to slowly drain back to the core *defs.js and services as things are updated and moved to separate repos. We've worked out some of the details of something like this already, so I may end up picking it up soon-ish... with results in one month to ten years. :8ball:
    --- SBBSecho 3.14-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)
  • From Prime@1:103/705 to Deuc¿ on Fri Apr 9 19:21:15 2021
    Re: Formatting for JS/SSJS files
    By: Deuc¨ to GitLab note in main/sbbs on Sat Apr 03 2021 12:36 am

    https://gitlab.synchro.net/main/sbbs/-/issues/246#note_1887

    Tab width shouldn't matter as long as you use spaces for alignment. If prettier can't use spaces for alignment, I suggest using a tool that can.

    Spaces for alignment instead of tab characters is just _EVIL_... Shudder... :-p

    ---
    þ Synchronet þ RetroConnect.org - Yet Another Glorified Offline Message Reader
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)