Skip to content

Conversation

PartyWumpus
Copy link
Contributor

Adds the typst-concealer plugin for displaying typst inline.

This is marked as draft because I'm not sure about:

  • Do I need to put something in some release notes?
  • Should I add default keybinds? The plugin basically needs them.
  • Should I change the descriptions to better fit the way they're phrased elsewhere? Currently they're just ripped from my code.
  • Should I be explicit about the defaults instead of just null and letting the plugin pick?

@NotAShelf
Copy link
Owner

Do I need to put something in some release notes?

A very quick entry would be nice.

Should I add default keybinds? The plugin basically needs them.

Sure, you can set them as its done in many module. See telescope for an example.

Should I change the descriptions to better fit the way they're phrased elsewhere? Currently they're just ripped from my code.

Option descriptions are usually from plugin documentation in our case. You're free to change them, but I'm not going to ask you to.

Should I be explicit about the defaults instead of just null and letting the plugin pick?

Leaving setupOpts empty means plugin's defaults will be used (if the plugin sets them.) Up to you whether you think some defaults are more... equal than others.

github-actions bot pushed a commit that referenced this pull request Jan 27, 2025
Copy link

github-actions bot commented Jan 27, 2025

🚀 Live preview deployed from b7571df

View it here:

Debug Information

Triggered by: NotAShelf

HEAD at: main

Reruns: 1409

github-actions bot pushed a commit that referenced this pull request Jan 27, 2025
@PartyWumpus PartyWumpus marked this pull request as ready for review January 27, 2025 14:56
@PartyWumpus PartyWumpus requested a review from NotAShelf January 27, 2025 14:56
@NotAShelf NotAShelf force-pushed the main branch 5 times, most recently from 02ee4cc to bc978c4 Compare March 17, 2025 11:42
Copy link
Collaborator

@Soliprem Soliprem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assuming you tested it and it works fine, this LGTM. Just need to fix conflicts with main @PartyWumpus

@PartyWumpus
Copy link
Contributor Author

PartyWumpus commented Sep 17, 2025

Sorry it took so long to get back to this, I didn't notice the ping 😭.

Fixed up the PR, but I'm a little confused what to do about the "spelling error" here. Am I good to just add typ to the typos file or is there a more local way to opt out?
image

@PartyWumpus
Copy link
Contributor Author

oh and yes i've tested this locally with typst.enable = true & typst.extensions.typst-concealer.enable = true, and it works as it should.

@NotAShelf
Copy link
Owner

Fixed up the PR, but I'm a little confused what to do about the "spelling error" here. Am I good to just add typ to the typos file or is there a more local way to opt out?

Please add it to the typos config so that the CI doesn't get triggered each time we commit to the repo.

Copy link
Collaborator

@Soliprem Soliprem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now

@NotAShelf NotAShelf merged commit b7571df into NotAShelf:main Sep 19, 2025
14 checks passed
@NotAShelf
Copy link
Owner

NotAShelf commented Sep 19, 2025

Thank you wumpus :)

Copy link

✅ Preview has been deleted successfully!

github-actions bot pushed a commit that referenced this pull request Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants