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: Virtual macro files #19130

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

Veykril
Copy link
Member

@Veykril Veykril commented Feb 10, 2025

Very WIP

This allows a client to opt-into functionality to treat macro expansions as virtual files (something r-a already does, but not yet exposes). Notably this means go to def onto a reference to a definition defined within a macro expansion will now jump into the macro expansion (when the feature is enabled).

This means upmapping has to be handled by the caller (the one who invokes the IDE features now), making that property lazy in a sense. The one annoyance is that this exposes MacroFileIds which are not necessarily stable.

Some missing things:

  • ability to opt-into this behavior
  • handling of stale macro ids (will currently result in panics)
  • change notification
  • formatting
  • Restructure NavigationTarget wrt to upmapping behavior
  • Update the semantics token descension API to allow for macro file inputs
  • refactor for performance

Follow up feature to this to be implemented, new command to enter into a macro expansion

(see https://bsky.app/profile/lukaswirth.bsky.social/post/3lhqqelx6hs2h for a first prototype)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 10, 2025
@ChayimFriedman2
Copy link
Contributor

This is extremely cool! Although without formatting the files are pretty unreadable, you can still see IDE features work in them.

BTW, with this feature does search search in macro expansions as well?

@Veykril
Copy link
Member Author

Veykril commented Feb 10, 2025

This is extremely cool! Although without formatting the files are pretty unreadable, you can still see IDE features work in them.

Yes, unfortunately we can't use rustfmt, but I believe we can at least fix up some whitespace and record those whitespace insertions to map things, that should make it a bit nicer to look at.

BTW, with this feature does search search in macro expansions as well?

In theory we could do that, in practice it would make everything crawl to a halt. (we'd need to collect all relevant macro calls and well then search through all of them)

@ChayimFriedman2
Copy link
Contributor

unfortunately we can't use rustfmt

Why not? I thought about maintaining some kind of mapping, token range before formatting to range after formatting, and using it in all features. Creating the mapping should be easy enough given that the output is just the input plus some whitespaces.

@Veykril
Copy link
Member Author

Veykril commented Feb 10, 2025

Creating the mapping should be easy enough given that the output is just the input plus some whitespaces.

Is it? I am not sure it is. The other issue is that we preserve $crate in this view now (also our custom builtin# syntax) which rustfmt cannot understand making it refuse to format. I won't stop you from trying though, it would be great if that worked. Right now I am getting our prettify infra to do the job as an MVP.

Comment on lines 25 to 129
pub(crate) transform: PositionTransform,
}

impl LineIndex {
pub(crate) fn line_col(&self, mut offset: TextSize) -> ide::LineCol {
if !self.transform.insertions.is_empty() {
offset += TextSize::new(
self.transform
.insertions
.iter()
.copied()
.take_while(|&(off, _)| off < offset)
.map(|(_, len)| ws_kind_width(len))
.sum::<u32>(),
);
}
self.index.line_col(offset)
}

pub(crate) fn line(&self, line: u32) -> Option<ide::TextRange> {
let mut range = self.index.line(line)?;
if !self.transform.insertions.is_empty() {
let mut iter = self.transform.insertions.iter();
let mut nl_seen = 0;
let overall_sub = TextSize::new(
iter.peeking_take_while(|&&(_, ws)| {
let m = nl_seen != line;
if ws == PrettifyWsKind::Newline {
nl_seen += 1;
}
m
})
.copied()
.map(|(_, len)| ws_kind_width(len))
.sum::<u32>(),
);
let end_sub = TextSize::new(
iter.copied()
.take_while(|&(_, ws)| ws != PrettifyWsKind::Newline)
.map(|(_, len)| ws_kind_width(len))
.sum::<u32>(),
);
range =
TextRange::new(range.start() - overall_sub, range.end() - overall_sub - end_sub);
}
Some(range)
}
}

#[derive(Default)]
pub struct PositionTransform {
pub insertions: Vec<(TextSize, PrettifyWsKind)>,
}

fn ws_kind_width(ws: PrettifyWsKind) -> u32 {
match ws {
PrettifyWsKind::Space => 1,
PrettifyWsKind::Indent(indent) => 4 * (indent as u32),
PrettifyWsKind::Newline => 1,
}
Copy link
Member Author

Choose a reason for hiding this comment

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

I messed up my commits so the whitespace fixup mapping stuff didnt get its own commit unfortunately but this is the relevant part (super in efficient as one can tell as I hacked it together real quick). There is also an off by one error here somewhere as some offsets seem to not map cleanly causing things to break

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants