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

feat: add decorations library with brace function #227

Merged
merged 3 commits into from
Sep 30, 2023
Merged

feat: add decorations library with brace function #227

merged 3 commits into from
Sep 30, 2023

Conversation

RubixDev
Copy link
Contributor

This PR adds a new CeTZ library called "Decorations". Currently it only contains one function, i.e. braces, but I thought other decorations may also be added to the same module in the future. For example, zig-zag and coiled lines, or more complex shapes like gears.

Here is a screenshot of the example I added to the manual:

image

A few points you may want to consider before merging:

  • The brace function does not define a custom style root, and because it uses merge-path internally also doesn't even use the bezier styles. I think it would be better for users if they could set styles for all braces, also including the things that are currently direct function parameters (amplitude, pointiness, content-offset). I just don't have enough experience with CeTZ yet to implement that myself.
  • The brace function takes a debug parameter. Should this maybe just respect the canvas's debug setting (if that is even possible)? It shows more specific and different information than the normal debug though.
  • The brace function currently does no type and value checking of the parameters. Should some assertions be added?
  • The brace function exposes anchors for all debug points. I guess this could potentially expose a bit too much of the internal structure and may lead to breaking changes for just slight internal refactors. The most important points are already exposed with appropriate names, so I would have no problem with removing these anchors.
  • I just saw that there is a line-normal function in the util module, which could be used in two places instead of the _rotate-around function and may be faster or more accurate.
  • The current description of the module in the manual is a bit meh, but I don't really know what to write there with only one function existing yet.

Also just for reference, this PR started out with me experimenting on the Typst Discord here.

Copy link
Member

@fenjalien fenjalien left a comment

Choose a reason for hiding this comment

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

This is really impressive nice work!

  • Have a look at the other libs to see how they resolve their styles
  • If you wrap the whole thing in a group and use a callback you can get ctx which has ctx.debug so you could attach it to that. You can keep the parameter anyway as it is cool to see.
  • Have added a comment regarding type checking, main values to focus are coordinates. You can do it without ctx by using coordinate.resolve-system(...) which will just return a string of the coordinate type or panics if it is invalid.
  • Don't worry about the debug anchors they're cool.
  • If you can use something that aready exists in then yea, if not dw.
  • Its alright.

Could you add a test for them? we don't have any guidlines but @johannes-wolf would be able to help if you need it.

src/lib/decorations.typ Outdated Show resolved Hide resolved
src/lib/decorations.typ Outdated Show resolved Hide resolved
src/lib/decorations.typ Outdated Show resolved Hide resolved
src/lib/decorations.typ Outdated Show resolved Hide resolved
src/lib/decorations.typ Outdated Show resolved Hide resolved
src/lib/decorations.typ Outdated Show resolved Hide resolved
src/lib/decorations.typ Show resolved Hide resolved
src/lib/decorations.typ Outdated Show resolved Hide resolved
@Beiri22
Copy link

Beiri22 commented Sep 30, 2023

That is exactly what I need; please, when you merge this PR, push it as new version to the package repo!

@johannes-wolf johannes-wolf added the feature 🎁 Feature Request label Sep 30, 2023
@johannes-wolf johannes-wolf added this to the 0.1.3 milestone Sep 30, 2023
@johannes-wolf
Copy link
Member

johannes-wolf commented Sep 30, 2023

That is exactly what I need; please, when you merge this PR, push it as new version to the package repo!

Ok, will release 0.1.3 after this got merged.

@johannes-wolf
Copy link
Member

This looks great! Cant wait to have this merged :)

sorry, I should've made multiple commits
Copy link
Member

@johannes-wolf johannes-wolf left a comment

Choose a reason for hiding this comment

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

LGTM.
I will add tests for this in a follow-up PR.

@johannes-wolf johannes-wolf merged commit 37f32bf into cetz-package:master Sep 30, 2023
1 check passed
@johannes-wolf
Copy link
Member

@Beiri22 Release PR: typst/packages#165

@RubixDev RubixDev mentioned this pull request Oct 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🎁 Feature Request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants