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

Add get-type command #730

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

TedDriggs
Copy link
Contributor

The get-type command (requested in #620) takes a line and character number which point to a complete identifier in the input text. The command will then attempt to determine the concrete type of that identifier.

Note that this also includes the travis updates from #728; this was necessary to make CI pass.

The `get-type` command (requested in racer-rust#620) takes a line and character number which point to a complete identifier in the input text. The command will then attempt to determine the concrete type of that identifier.
Copy link
Collaborator

@jwilm jwilm left a comment

Choose a reason for hiding this comment

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

Hey this is really cool! Thanks 😄

Looks good overall, but had a few questions about match strings/determined types.

";

let got = get_type(src, None);
assert_eq!("Option", got.matchstr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, can we not determine that it's an Option<usize>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't put generic types into the matchstr property today; they're in the generic_types array instead. I could try and do something to push that data into matchstr but I'm not sure which approach would be better for tooling authors.

"#;

let got = get_type(src, None);
assert_eq!("str", got.matchstr);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't this be &str? name is a reference type (ie, a pointer).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MadRubicant
Copy link

Is there any reason this PR hasn't been merged?

@kngwyu
Copy link
Collaborator

kngwyu commented Aug 17, 2018

@MadRubicant
Simply it needs rebasing, and I'm not sure it's so useful for clients.
But if you need this feature, I'm going to work on this.

@MadRubicant
Copy link

I don't specifically need this feature; I was hoping it would let me hack together completion for closures. I cloned the branch to test it, and trying to get the type of a closure argument causes a panic

@kngwyu
Copy link
Collaborator

kngwyu commented Aug 17, 2018

@MadRubicant
Then this branch is too old to hack 😟
I added support for closure arg in #867, but it needs type annotation 😫
Any contribution for enhancement is welcome, but please use the latest mater branch 🙂

@shinzui
Copy link

shinzui commented Oct 6, 2018

I would love to have this feature. I have to resort to hacks to get the type of a variable in neovim now.

@kngwyu kngwyu self-assigned this Oct 7, 2018
@kngwyu
Copy link
Collaborator

kngwyu commented Oct 7, 2018

@shinzui
Thanks for feedback.
I assigned myself.

@kngwyu kngwyu mentioned this pull request Oct 23, 2018
10 tasks
@Baranowski
Copy link

Hi @kngwyu, has there been any progress on this feature?

I got super excited when I saw this PR, and then I realized how old it was. I would be happy to try to rebase/rewrite it on top of master, if nobody else is working on this and if this functionality is not supplied somewhere else (e.g. rustc).

@kngwyu kngwyu removed their assignment Oct 14, 2019
@kngwyu
Copy link
Collaborator

kngwyu commented Oct 14, 2019

@Baranowski
Sorry, I'm not so active in this project now.
It's great if you want to work for this PR, though there are some other active projects like rust-analyzer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants