-
Notifications
You must be signed in to change notification settings - Fork 495
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
LS: Proc macro - scarb code #6638
base: spr/main/36b6c854
Are you sure you want to change the base?
Conversation
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.
Reviewed 7 of 7 files at r1, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, @mkaput, and @orizi)
crates/cairo-lang-language-server/src/lang/proc_macros/plugins/scarb/regular.rs
line 30 at r1 (raw file):
} fn calculate_metadata(db: &dyn SyntaxGroup, item_ast: ast::ModuleItem) -> TokenStreamMetadata {
This should be lower (public first)
6b63f90
to
55303af
Compare
94ce188
to
3709348
Compare
55303af
to
e49ac1f
Compare
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.
Reviewable status: 3 of 7 files reviewed, all discussions resolved (waiting on @Arcticae, @mkaput, @orizi, and @piotmag769)
crates/cairo-lang-language-server/src/lang/proc_macros/plugins/scarb/regular.rs
line 30 at r1 (raw file):
Previously, piotmag769 (Piotr Magiera) wrote…
This should be lower (public first)
Done.
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.
Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Arcticae, @mkaput, and @orizi)
e49ac1f
to
1790b0d
Compare
f7c5016
to
bedb75d
Compare
f1a8998
to
cb0d2d5
Compare
4966290
to
1c0f71e
Compare
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.
Reviewed 1 of 2 files at r4, all commit messages.
Reviewable status: 5 of 7 files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, @orizi, and @piotmag769)
crates/cairo-lang-language-server/Cargo.toml
line 29 at r4 (raw file):
cairo-lang-utils = { path = "../cairo-lang-utils", version = "~2.8.4" } cairo-lang-macro = "0.1.1" # Used in proc-macro code copied from scarb.
These comments are unnecessary imho: if we use these deps, then we use them, and comments break easily sorting deps
1c0f71e
to
ae851a5
Compare
cb0d2d5
to
4763ba9
Compare
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.
Reviewable status: 3 of 7 files reviewed, 1 unresolved discussion (waiting on @Arcticae, @mkaput, @orizi, and @piotmag769)
crates/cairo-lang-language-server/Cargo.toml
line 29 at r4 (raw file):
Previously, mkaput (Marek Kaput) wrote…
These comments are unnecessary imho: if we use these deps, then we use them, and comments break easily sorting deps
Done.
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.
Reviewed 1 of 4 files at r2, 2 of 2 files at r4.
Reviewable status: 4 of 7 files reviewed, 5 unresolved discussions (waiting on @Draggu, @mkaput, @orizi, and @piotmag769)
crates/cairo-lang-language-server/src/lang/proc_macros/plugins/scarb/inline.rs
line 12 at r4 (raw file):
use crate::lang::proc_macros::db::ProcMacroCacheGroup; // scarb code
I would mark where it comes from, for easier exploration and maintainability (perhaps a git permalink also would be nice)
crates/cairo-lang-language-server/src/lang/proc_macros/plugins/scarb/mod.rs
line 11 at r4 (raw file):
pub mod regular; // Code from scarb.
Same as above
crates/cairo-lang-language-server/src/lang/proc_macros/plugins/scarb/regular.rs
line 20 at r4 (raw file):
use crate::lang::proc_macros::db::ProcMacroCacheGroup; // scarb code
Same as above
crates/cairo-lang-language-server/src/lang/proc_macros/plugins/mod.rs
line 12 at r4 (raw file):
// Keep it private. mod downcast; // Code from scarb.
Let's not describe it on import
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.
Reviewed 1 of 1 files at r3, 2 of 2 files at r5, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Draggu and @mkaput)
4763ba9
to
597ba75
Compare
597ba75
to
e1bfa43
Compare
Code calling dylib is replaced with one calling proc-macro-server. Also `aux_data` is removed as it is useless for LS. commit-id:381d1f35
37c53d9
to
dcdb5d7
Compare
e1bfa43
to
905c785
Compare
Code calling dylib is replaced with one calling proc-macro-server. Also
aux_data
is removed as it is useless for LS.Stack:
select!
#6701