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

Taplo Integration Tracking #15908

Closed
wants to merge 2 commits into from

Conversation

alibektas
Copy link
Member

This PR is used for tracking how far we have gotten in terms of integrating taplo, which we will use as our rowan based TOML parser.

Related issues are #13529 and #15741

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 16, 2023
@flodiebold
Copy link
Member

Two notes:

  • for rust-analyzer.toml files, do we want to do the parsing in the HIR crates? I would see configuration loading and parsing as an ide-level concern and expect it to live in the ide or project-model crates (and any analysis-relevant settings would be passed from there, as happens now). It's also worth noting that at least I would not expect changes to rust-analyzer.toml to take effect immediately when you edit the file, rather only when you explicitly save, which is contrary to how the VFS and Salsa work. Also, when using RA crates as a library, one might want to control these settings in a different way.
  • apart from that, for Cargo.toml, do we want the related code to live in syntax etc., or should they have their own crates?

@Veykril
Copy link
Member

Veykril commented Nov 17, 2023

apart from that, for Cargo.toml, do we want the related code to live in syntax etc., or should they have their own crates?

I'd say we should have a separate crate for that, the syntax crate shouldn't really have anything in it that is of much use to the toml stuff I think. syntax should really only be bothering with rust syntax in my opinion.

@flodiebold
Copy link
Member

Actually thinking about it a bit more, we might later want to have rust-analyzer.toml in the VFS to provide IDE features while editing them, but should still only load the actual settings from disk 🤔

@bors
Copy link
Contributor

bors commented Dec 7, 2023

☔ The latest upstream changes (presumably #16043) made this pull request unmergeable. Please resolve the merge conflicts.

Given keys, we can search for them within a given str.
Right now this is possible for RATOML case, which means
that we need to have a unified solution for both RATOML and Cargo.toml
@Veykril Veykril added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 3, 2024
@bors
Copy link
Contributor

bors commented Jan 24, 2024

☔ The latest upstream changes (presumably #16420) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Feb 1, 2024

☔ The latest upstream changes (presumably #16451) made this pull request unmergeable. Please resolve the merge conflicts.

@Veykril
Copy link
Member

Veykril commented Feb 12, 2024

I'll close this PR. A tracking PR doesn't make much sense, we should instead use a tracking issue for that. The changes in this PR also don't seem to be too useful right now.

@Veykril Veykril closed this Feb 12, 2024
@alibektas alibektas deleted the taplo_integration branch February 12, 2024 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants