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

Avoid linting typst colors #854

Closed

Conversation

alex-tdrn
Copy link

Issues

#849

Description

Avoid linting typst calls to color.rgb("...")/rgb("...") since they represent hex codes. Accomplish this by ignoring any call to the rgb global function and any member access to the color global module. A typst user could theoretically shadow the color module and/or the rgb global function with their own implementation and add string parameters that represent text that should be checked for grammar/typos, but surely no reasonable user would do that.

If this is seen as a good strategy, I think it could be used to tackle #846 as well as other such "do not check the string parameter of this specific function since it is not prose" cases.

I have also added 2 extra unrelated commits to make life a bit easier for contributors who:

  • work on the typst files and have some kind of language server that automatically renders pdfs in the background and might commit them by accident
  • work on windows and might commit CRLF files by accident

How Has This Been Tested?

I have added a test file and I have built and tested harper with this change on personal typst projects that use the rgb function.

Checklist

  • I have performed a self-review of my own code
  • I have added tests to cover my changes

Typst LSPs such as `tinymist` can be set up to automatically generate
a PDF of a `.typ` file on save, which can lead to random PDFs being
generated when editing/adding test files to `harper-typst`.
Contributors who work on Windows might accidentally commit newly
created files with CRLF line endings, later causing noisy diffs
when Linux developers work on the same files and they do not have
`core.autocrlf = true`.
However nowadays, most text editors work with LF line endings just
fine on windows, so there's no need for this incompatibility.

Add an `.editorconfig` file which causes the text editor to create
new files using LF from the get-go instead of CRLF (VSCode needs
https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig
in order to conform).

Add a `.gitattributes` file to tell git to always checkout files using
LF as line ending.

Normalize line endings throughout the codebase using
`git add --renormalize .`.

https://adaptivepatchwork.com/2012/03/01/mind-the-end-of-your-line/
https://docs.github.com/en/get-started/git-basics/configuring-git-to-handle-line-endings
@elijah-potter
Copy link
Collaborator

elijah-potter commented Mar 8, 2025

Hey @grantlemons, would you mind taking a look at this? I'm not as well-versed in the Typst spec.

@grantlemons
Copy link
Collaborator

grantlemons commented Mar 8, 2025

@elijah-potter #857

grantlemons added a commit to grantlemons/harper that referenced this pull request Mar 8, 2025
grantlemons added a commit to grantlemons/harper that referenced this pull request Mar 8, 2025
@alex-tdrn
Copy link
Author

@elijah-potter #857

Well I guess I'm going to close this PR in favor of that one. Would have appreciated a review, though, so I can make better contributions in the future, here. @elijah-potter feel free to cherry-pick the first 2 commits if they are useful to you.

@alex-tdrn alex-tdrn closed this Mar 8, 2025
@grantlemons
Copy link
Collaborator

@alex-tdrn Ahh so sorry that I just kinda took this over, for some reason I thought this was an issue not a PR!

@alex-tdrn
Copy link
Author

@alex-tdrn Ahh so sorry that I just kinda took this over, for some reason I thought this was an issue not a PR!

No worries then! I like your solution more anyway, seems easier to extend with more special cases in the future 👍

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.

3 participants