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

Added resolve submodules with raw name #1478

Merged
merged 3 commits into from Jul 3, 2019

Conversation

@andreevlex
Copy link
Contributor

commented Jul 3, 2019

@lnicola

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

Can you take and return a slice instead to avoid the allocation?

@andreevlex

This comment has been minimized.

Copy link
Contributor Author

commented Jul 3, 2019

struct Name support get slice?

@lnicola

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

Ah, sorry. I was looking at this from my phone.

@@ -662,6 +663,14 @@ fn resolve_submodule(
}
}

fn resolve_mod_name(name: String) -> String {

This comment has been minimized.

Copy link
@matklad

matklad Jul 3, 2019

Collaborator

I think we shouldn't do this here. Rather, the fact that Name contains an r# is a bug: Name is supposed to be clean, semantic name.

I think this cleaning should happen in these two impls:

impl AsName for ast::NameRef {
fn as_name(&self) -> Name {
Name::new(self.text().clone())
}
}
impl AsName for ast::Name {
fn as_name(&self) -> Name {
Name::new(self.text().clone())
}
}

Move resolve raw name in name.rs
Added test for check module resolution with raw name
@matklad
Copy link
Collaborator

left a comment

Looks good, left a couple of tiny suggestions!

bors delegate+

} else {
text
}
}

This comment has been minimized.

Copy link
@matklad

matklad Jul 3, 2019

Collaborator

Two nits:

  • it seems cleaner to accept &SmolStr and invoke .clone, rather than just drop an owned value in case of r#
  • let's name this cook_name: that's what you do to process raw things

This comment has been minimized.

Copy link
@andreevlex

andreevlex Jul 3, 2019

Author Contributor

Fixed

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

✌️ andreevlex can now approve this pull request

@matklad

This comment has been minimized.

Copy link
Collaborator

commented Jul 3, 2019

bors r+

Thanks!

bors bot added a commit that referenced this pull request Jul 3, 2019

Merge #1478
1478: [WIP] Added resolve submodules with raw name r=matklad a=andreevlex

#1211

Co-authored-by: Alexander Andreev <andreevlex.as@gmail.com>

@andreevlex andreevlex changed the title [WIP] Added resolve submodules with raw name Added resolve submodules with raw name Jul 3, 2019

@bors

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2019

@bors bors bot merged commit 6263aa1 into rust-analyzer:master Jul 3, 2019

2 checks passed

bors Build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@andreevlex andreevlex deleted the andreevlex:feature/resolve-raw-mods branch Jul 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.