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

Auto formatting #1960

Merged
merged 8 commits into from
Jan 24, 2024
Merged

Auto formatting #1960

merged 8 commits into from
Jan 24, 2024

Conversation

egekorkan
Copy link
Contributor

@egekorkan egekorkan commented Jan 23, 2024

Takes over #1956

I had to also fix some HTML errors like missing closing tags etc.


Preview | Diff

@egekorkan
Copy link
Contributor Author

egekorkan commented Jan 23, 2024

To make it easier to review, here is a summary of changes. I am writing it down but I will update this comment with the final state.

  1. The "add render script results" commit (46dd37e) only contains changes that happen after rendering.
  2. Markdown lists are surrounded with empty lines.
  3. Markdown headings get an empty line before the heading
  4. Single quotes are turned into double quotes
  5. ** is used instead of __ for bold emphasis in markdown
  6. _ is used instead of * for italic emphasis in markdown
  7. - for markdown listing instead of *
  8. [{ constructs are split into two lines
  9. Space between : and { in JSON, e.g. "fade": { instead of "fade":{
  10. Lines longer than 120 characters are split into multiple lines. Similarly, if a line can be shorter than 120 lines, it is merged into one.
  11. Unnecessary empty lines are deleted
  12. JavaScript arrow function variable always has parantheses, e.g. filter((p) => p) instead of filter(p => p)
  13. Semicolon ending is enforced
  14. Enforcing 2 spacing for tabulation
  15. Respecting indentation with each encapsulation

Notes:

  • Even though this is a big diff, it should reduce diff size in the future.
  • It is not possible to split this PR since the formatting has to apply everywhere.
  • As this is just beautification, no change should happen that affects a visible or noticable result. Indeed, index.html almost doesn't change in the linked rendered diff above.
  • I have reviewed the changed files but others are welcome.

@egekorkan egekorkan linked an issue Jan 23, 2024 that may be closed by this pull request
@egekorkan
Copy link
Contributor Author

egekorkan commented Jan 24, 2024

In the TD call, we noticed the following changes:

  1. We should add gitattribute file so that the first clone is using the correct line ending.
  2. Having trailing comma in code without inline JSON would be better. So we can add prettierconfig in folders with just "normal" code or use // prettier-ignore inline. This avoids unintended diffs. This will be another follow up discussion.

@egekorkan will do the first point asynchronously and then merge the PR.

@egekorkan
Copy link
Contributor Author

@relu91 I have added the gitattributes file as well. I have taken the one from node-wot but added some other file endings listed below:

  • .puml
  • .sparql
  • .html
  • .ttl
  • .yml

I am merging the PR now. If there are other files I have missed, please provide a follow-up PR :)

@egekorkan egekorkan merged commit 791858c into main Jan 24, 2024
1 check passed
@egekorkan egekorkan deleted the ege-formatting-2 branch February 21, 2024 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Aligning formatting across files
1 participant