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

Add formatter #526

Closed
FireIsGood opened this issue May 10, 2024 · 3 comments · Fixed by #538
Closed

Add formatter #526

FireIsGood opened this issue May 10, 2024 · 3 comments · Fixed by #538

Comments

@FireIsGood
Copy link
Contributor

Over all the pages and layout files, lots of spacing and minor changes are inconsistent. One solution to this might be a formatter to run over the code.

One option I would lean towards would be a more opinionated one such as Prettier. I have made a branch on my local fork to show what this would look like.

Lint commit example: FireIsGood@aade527

Changes might be as follows:

Header style changes
(underline dash to prefix pound sign)

- 
- Getting Started
- --------
+ ## Getting Started

Frontmatter changes
(consistent quotation type)

- title: 'Cheatsheet request: '
+ title: "Cheatsheet request: "

Table fixes
(Formatted nicely!)

- Shortcut | Action
- ---|---
- `K/J`  | Move Up/Down
- `H`  | Jump to Inbox tab
- `S`  | Jump to Archive tab
- `F`  | Follow or Unfollow
- `I`  | Archive
- `U`  | Move to Inbox
+ | Shortcut | Action              |
+ | -------- | ------------------- |
+ | `K/J`    | Move Up/Down        |
+ | `H`      | Jump to Inbox tab   |
+ | `S`      | Jump to Archive tab |
+ | `F`      | Follow or Unfollow  |
+ | `I`      | Archive             |
+ | `U`      | Move to Inbox       |
+ 

Concerns

Currently the style guide recommends using a full underline of dash characters - to make the h2 level headings. The example of prettier would have all of these converted to using the two pound sign prefix version.

Some assorted files have slight formatting errors that may trip up the formatter. One example I found was the chatgpt file which had some very specific markdown code block issues due to its specific format.

No formatted:

- **Choose a random contest winner(s) from a long list of names or emails**
    ``` {.wrap}
    I want to choose a winner from a list of 100 names, can you help?
    ```
    ``` {.wrap}
    Can you randomly pick 5 email addresses from a list of 1000 for a giveaway contest?
    ```
{.collapsible}

Automatic format:

- **Choose a random contest winner(s) from a long list of names or emails**
  `{.wrap}
 I want to choose a winner from a list of 100 names, can you help?
`
  `{.wrap}
 Can you randomly pick 5 email addresses from a list of 1000 for a giveaway contest?
`
  {.collapsible}

Required Changes

If the project were to adopt something like prettier, a new script in the package.json would have to be made such as format. To make sure it runs on every commit, a tool such as Husky could be employed pretty simply, though that would also make users sit through a ~7 second commit process every commit as the program scanned all the files.

The style guide may have to be changed slightly, though it seems that due to embedding other files it would be fine from a brief glance.

A full review of the changes would be required for the first lint, though subsequent lints would be fixed by the authors.


If this sounds like a good idea, I could make a pull request with all the proposed changes.

@Fechin
Copy link
Owner

Fechin commented May 16, 2024

Thank you for your suggestion, I think it's great. Prettier can help maintain code cleanliness. I look forward to seeing your PR. As you mentioned, a thorough review of the changes would be necessary for the initial lint.

@FireIsGood
Copy link
Contributor Author

I've begun work on this and it seems that the tons of legacy code are messing up the automatic formatting powers. I've done a bunch of manual edits and it seems like around half of them will need manual fixing either for existing issues or issues that appear prettier is run.

Also, I was not sure exactly what Prettier settings you would prefer. Since it is an opinionated formatter, I set it with minimal defaults except for a print width of 120 characters (matches the Laravel lines of text in the README wrapping, though with a more conventional wrap count) and always wrapped the prose.

On the subject of making sure further commits actually follow the linting, do you think adding something like Husky would be a good idea for that pull request, another pull request, or not at all? Without a tool like that, users would have to remember to run npm run format after changes or others will have errors not caused by them.

Here's the commits of the branch if you want to track my progress: https://github.com/FireIsGood/reference/commits/prettier/

@FireIsGood
Copy link
Contributor Author

Another note, a lot (around half so far) have had other issues that Prettier was causing.

I have been in the process of manually editing every single post file while merging changes, so it may be a while until the full pull request is done, so this Issue is being worked on.

Additionally, I plan to make a pull request for the full prettier format which will format all the source code as well as posts, then make a separate pull request to add the tool Husky as a pre-commit hook to make sure everyone must format their posts.

...

Tl;dr: issue is being worked on, it's a lot, there will be two pulls.

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 a pull request may close this issue.

2 participants