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

Code formatting #64

Open
abisammy opened this issue Jun 13, 2023 · 8 comments
Open

Code formatting #64

abisammy opened this issue Jun 13, 2023 · 8 comments

Comments

@abisammy
Copy link
Contributor

abisammy commented Jun 13, 2023

Currently the code has to be styled manually, I would recommend we use a library to format it, such as prettier.

This can automatically format the code to how you like, this would be an example config:

{
    "printWidth":120,
    "useTabs":true,
    "semi":true,
    "singleQuote":true,
    "trailingComma":"all",
    "bracketSpacing":false
}

Prettier can be installed globally using npm, the config would then be placed in the directory of the project, and when running the command prettier --write "*.js", it will automatically format all the files for us!

@Moon-0xff
Copy link
Owner

I've been thinking of removing the quirks of the style and align more strictly with gnome-shell style guide.

I haven't even mentioned it because:

  • Committing a change like that will hinder the usefulness of git blame
  • It's work!
  • The corner cases aren't likely to be obvious
  • I do actually prefer the current style.

@Moon-0xff
Copy link
Owner

I don't know this prettier tool/library, it is a simple indentation/style helper?
I don't want to add IDE config files, or external libraries to the repository.

If prettier is a widely known, stable and easy to set up tool, while also requiring just a short config like the example I don't see any issue of adding the config and explanation to HACKING.md

@Moon-0xff
Copy link
Owner

Moon-0xff commented Jun 13, 2023

Also, I wouldn't trust a format tool to re-style the code, no problem using it as a tool to reformat the code, but it will need to be checked side by side, I expect a few corner cases that we will need to resolve either by coming up with something or searching for a similar code in gnome-shell

Although, the code is just above 1000SLOC so it shouldn't be a difficult task.

Note: This is the gnome-shell style guide, or at least the one I know of
Note2: git probably has a way of doing this without messing up the commit history

@abisammy
Copy link
Contributor Author

abisammy commented Jun 13, 2023

Prettier is a code formatter, this means it can format the source code directly to ensure consistency, for example:

  • It can ensure code is properly indented
  • It can ensure that strings use single quotes
  • It can automatically add semicolons to the end of this line

Prettier is well known, it has 30 million weekly downloads on NPM
It is typically used with NodeJS projects (a javascript runtime, that has sort of become the standard for building websites).
Prettier also provides a command line interface to format code however, which is what I propose we do.

Prettier can also be setup with an IDE, for example vscode to format the code whenever saving a file, however it is not neccessary.

Prettier is very stable, and I have used it for a while, and never experienced any problems with it changing the logic of code, it simply reformats it. Prettier also will refuse to format the code if it contains a syntax error (for example missing a closing brackets), so does not break code that has not been completed.

Prettier's customisability can be a bit limited, however this is a test branch I published with the code using an example config that is close to the original. I recommend looking at the final code, in addition to the diff, as you can more easiliy see the final formatted code, with a consistent style.

I understand the diff would be huge, however in the long run it will enforce code readabilty and style, as the same program would use the same config in order to format the code.

I'm not aware of other tools like it, there are some others that can format code, however require a more complicated setup or config, this is the simplest that I know of.

@Moon-0xff
Copy link
Owner

Well, as mentioned:

  • Merging that diff will hinder git blame tremendously, this is specially troublesome because our code isn't well documented
  • I don't want to add IDE config files to the repo, only as documentation
  • The output files need to be reviewed, which isn't a trivial task

@abisammy
Copy link
Contributor Author

Just to clarify, the config is a config for prettier itself (the command line utility), and not IDE specific.
Additionally what do you mean by reviewing the output please, does this mean reviewing the code has not been altered to break it? If so prettier will not change the logic, just the format of the source code?

@Moon-0xff
Copy link
Owner

Just to clarify, the config is a config for prettier itself (the command line utility), and not IDE specific.

If the IDE uses prettier, then it is.

Additionally what do you mean by reviewing the output please, does this mean reviewing the code has not been altered to break it? If so prettier will not change the logic, just the format of the source code?

I don't expect it to break the code, but might look wrong or not follow the coding style in some places.

@abisammy
Copy link
Contributor Author

If the IDE uses prettier, then it is.

The config can either be used by the IDE or, the command line tool, its a generic config for prettier.

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

2 participants