Skip to content

Make trivia first class member of token - #219

Draft
bit-aloo wants to merge 2 commits into
rust-analyzer:masterfrom
bit-aloo:2026-08-14-trivia-first-class
Draft

Make trivia first class member of token#219
bit-aloo wants to merge 2 commits into
rust-analyzer:masterfrom
bit-aloo:2026-08-14-trivia-first-class

Conversation

@bit-aloo

@bit-aloo bit-aloo commented Aug 14, 2026

Copy link
Copy Markdown
Member

Still making changes to RA

@ChayimFriedman2 ChayimFriedman2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know you opened this as draft by I've still reviewed it so you don't waste time changing r-a to match an API we'll throw away.

Comment thread src/trivia.rs Outdated

#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
pub enum TriviaPieceKind {
Newline,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It is not the job of rowan to define this. Trivias should just have a SyntaxKind. From rowan's point of view, they're tokens attached to tokens. The rest will be handled by rust-analyzer. This will also require fewer changes.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed.

Comment thread src/api.rs Outdated
self.raw.text()
}

pub fn text_trimmed(&self) -> &str {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe text() and text_range() should exclude trivia, and we should have a text_including_trivia() method (and ditto for text_range()). While I don't know for sure, code in r-a today does not handle trivia so I expect this to be easier.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I also think it'll be easier if trivias will store their text (like regular tokens).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yup, having both text and text_range with including trivia variants.

Comment thread src/green/token.rs Outdated

let start: usize = leading_len.into();
let end: usize = (total_len - trailing_len).into();
let text = unsafe { std::str::from_utf8_unchecked(self.data.slice()) };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
let text = unsafe { std::str::from_utf8_unchecked(self.data.slice()) };
let text = self.text();

Save the unsafe.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks

Comment thread src/green/trivia.rs Outdated
@@ -0,0 +1,134 @@
use std::{

@ChayimFriedman2 ChayimFriedman2 Aug 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not pleased with the amount of public functions here. We're going to remove green trees so we need to minimize their usage. Things that do not need to be public should remain private.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah!! sorry, lazy me. Should be good now.

Comment thread src/api.rs Outdated
}

impl<L: Language> SyntaxToken<L> {
pub fn new_detached<Leading, Trailing>(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why do we need this? A token cannot be a root, this feels wrong.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Removed. I was copy pasting from biome back then.

Comment thread src/cursor.rs Outdated
// endregion

#[derive(PartialEq, Eq, Clone, Hash)]
pub struct SyntaxTrivia {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we need this type. We can just have fn leading_trivia(&self) -> impl Iterator<Item = SyntaxToken> om SyntaxToken, and ditto for trailing_trivia(). Just store a list (even Box<[GreenToken]>) of leading trivia in a token, and ditto for trailing trivia.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In particular, using regular tokens for trivias simplifies both the implementation and the changes in r-a.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Its much better. Again, took inspiration from biome initially. Removed.

@bit-aloo
bit-aloo force-pushed the 2026-08-14-trivia-first-class branch from ebcb103 to c97bf3d Compare August 30, 2026 08:35
@bit-aloo
bit-aloo force-pushed the 2026-08-14-trivia-first-class branch from c97bf3d to 7e9daab Compare August 30, 2026 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants