-
-
Notifications
You must be signed in to change notification settings - Fork 57
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
Fix expressions indent #152
Conversation
Clippy seems to be complaining on a file that's not edited in this PR, probably a new version of Clippy is detecting some issues in the present written code ? |
I've raised a PR #154 to fix the jobs that are failing here. |
If you have the time do you mind reviewing this @wooorm ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience <3
use crate::{MdxExpressionKind, MdxExpressionParse, MdxSignal}; | ||
use alloc::boxed::Box; | ||
|
||
pub const INDENT_SIZE: usize = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub const INDENT_SIZE: usize = 4; | |
pub const INDENT_SIZE: usize = 2; |
There were some inconsistencies in the JS APIs, value is 2 now: https://github.com/micromark/micromark-extension-mdx-expression/blob/7c305ffb7dbce7a452c54d4f4a5fbd2e19652be0/packages/micromark-factory-mdx-expression/dev/index.js#L37
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In markdown-rs the indent size is 4
There's a comment which notes the above statement in here :
so I think there might be some subtlety here which i don't quite understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one more thing: changing the indent size in here will break other tests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, there are different places with different values. I managed to straighten that out in the JS world!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ll try my hand at this!
use crate::{MdxExpressionKind, MdxExpressionParse, MdxSignal}; | ||
use alloc::boxed::Box; | ||
|
||
pub const INDENT_SIZE: usize = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I’ll try my hand at this!
fixes #150
I'm not sure if that's the right approach to go about fixing this issue as this is the first time that I work on the compiler, hopefully it's :). Thanks