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

General improvements and fixes #44

Merged
merged 49 commits into from
Apr 2, 2024
Merged

Conversation

tecosaur
Copy link
Collaborator

I'm doing a big batch of changes to tighten up the current code and test it better, making a few fixes along the way.

Technically, styled"{():}" and styled"{( ):}" are legal instances of the
documented grammar.

Found by Supposition.jl.
Checking the value of the next char is a very frequent operation, and
one must always be careful to confirm that the stateful char iterator is
not empty before doing so. To improve readability, clarity, and make it
harder to forget this imported check, I'm introducing a helper function
for this task.
This catches two edge cases around the handling of space around inline
face attributes in the markup grammar.
In a similar style to the isnextchar introduction, this new predicate
function enhances the clarity of the markup parsing code.
At some point I just started typing "whitespace", when I meant to use
"ws", as "whitespace" isn't even defined anywhere in the grammar.
When making the macro-oriented markup parser into a hybrid
macro/function-oriented FSM, I looked around all the instances of `Expr`
to find code that needed updatig. In that process, this QuoteNode used
for weight was missed.
@tecosaur tecosaur force-pushed the general-improvements-and-fixes branch 18 times, most recently from ca795cb to e2adc15 Compare March 19, 2024 15:06
@tecosaur
Copy link
Collaborator Author

Progress update: I've added ~100 new tests over ~400 lines, and written a ~200 line fuzzer using Supposition with the help of @Seelengrab. 4 minor bugs have been fixed, one more major one (off by 1 error and unsigned integer overflow in the 8-bit colour approximation function).

All in all, I'm very happy with how this sprint of work is shaping up.

@tecosaur tecosaur force-pushed the general-improvements-and-fixes branch 3 times, most recently from f37b0d9 to 746adc6 Compare March 20, 2024 04:15
src/faces.jl Outdated Show resolved Hide resolved
@tecosaur tecosaur force-pushed the general-improvements-and-fixes branch 3 times, most recently from 3f04879 to 2966f9d Compare April 2, 2024 10:11
tecosaur and others added 9 commits April 2, 2024 18:14
Having it augment/replace the faces dict with the same method name seems
too potentially confusing. It is likely a better idea to copy the
withenv/setenv split if we want a "swap out the faces dict" method in
the future.
These two functions are designed as user-facing, and really should be
part of the advertised API surface (and the styled function already was
in the docs).
Based on feedback received from Sundar
This was the only aspect of inline face attributes that couldn't be
interpolated — a bit of an oversight.
@tecosaur tecosaur force-pushed the general-improvements-and-fixes branch 2 times, most recently from 3984156 to 5226f29 Compare April 2, 2024 10:21
Because it's invalid for a symbol to contain 0x00.

julia> Symbol("\0")
ERROR: ArgumentError: Symbol name may not contain \0
@tecosaur tecosaur force-pushed the general-improvements-and-fixes branch 5 times, most recently from 6d223be to d55fd33 Compare April 2, 2024 12:06
@tecosaur
Copy link
Collaborator Author

tecosaur commented Apr 2, 2024

Ok, I think we're at the point where this should be merged now.

@tecosaur tecosaur force-pushed the general-improvements-and-fixes branch 4 times, most recently from 6a84470 to 67f0ff6 Compare April 2, 2024 14:38
@tecosaur tecosaur force-pushed the general-improvements-and-fixes branch from 67f0ff6 to 9c015e2 Compare April 2, 2024 14:39
@tecosaur tecosaur merged commit 9c015e2 into main Apr 2, 2024
7 checks passed
@tecosaur tecosaur deleted the general-improvements-and-fixes branch April 2, 2024 16:13
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.

5 participants