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

attempt at implementing #10 #12

Closed
wants to merge 1 commit into from
Closed

Conversation

kianenigma
Copy link

This is my attempt at implementing #hidden. It is not working, and I think it is because my approach might fundamentally be incompatible with the intended way.

In short, what I did was to start parsing syn::File and remove any item/expr that comes after #[docify::hidden]. This part is intentioanlly left incomplete (it does not parse nested expressions like syn::Block).

Nonetheless, this is not working because source_excerpt (for reasons beyond my current understanding) was expecting to be fed the untouched source_code, not the modified version.

For a similar reason, all of my formatting was also being wiped. I like the current approach of always formatting the coment and I think it should be the default behavior. We can backport this part to master sooner.

@sam0x17
Copy link
Owner

sam0x17 commented Oct 13, 2023

ah yeah this would have to integrate closely with source_excerpt to work properly. At the top of my head I believe you would need to do your removals in a clean way that doesn't destroy formatting and then feed that into the regular docify parsing process. The CompressedString machinery has the tools you need to do this, as it lets you identify the "real" start and end position of an item within the original source string. If you take a look at the source code for source_excerpt it basically does exactly that using CompressedString

}

/// Process a syn::Item and its potential nested items
fn process_item(item: &mut Item) {
Copy link
Owner

Choose a reason for hiding this comment

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

would be much easier to use a visitor pattern for this, see https://docs.rs/syn/latest/syn/visit/index.html and other visitors in this file

Copy link
Author

Choose a reason for hiding this comment

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

I had it in the back of my head that doing this nested parsing probably has a proper algorithm somewhere and I should not need to re-invent it 😅 So it is the same visitor alg you have been talking about lately, thanks for sharing

Copy link
Owner

@sam0x17 sam0x17 Oct 19, 2023

Choose a reason for hiding this comment

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

In fact, it was my primary area of research in grad school and I've published some novel indexing data structures and algorithms around this problem that let you do it without having to have an awkward visitor pattern :)

@@ -209,7 +211,60 @@ trait AttributedItem {
fn set_item_attributes(&mut self, attrs: Vec<Attribute>);
}

impl AttributedItem for Item {
impl AttributedThing for Expr {
Copy link
Owner

Choose a reason for hiding this comment

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

btw, this is one of the things sin would make easier. Sooo many methods that should be provided by default but aren't

@sam0x17 sam0x17 closed this Oct 27, 2023
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.

2 participants