I was wondering if anyone had time to give a light review of #4873. It's a little bit behind master now, but I'd like to get some feedback before I rebase it if possible.
I’ll try to give a look tomorrow. * deps and map lit create create a „this is a wp“ impression for me, so I didn’t react immediately and then it just sitting in the queue. I should have reacted earlier...
Thanks, it's definitely a WIP, but I'd love to get some feedback :)
About code organisation you suggested creating a
resolve_doc_path method on types in code model. This would work but the code works with
Definition which is defined in
ra_ide_db. In order to avoid implementing the same method once for every variant of
Definition I'd need to expose something like the
Resolver in methods on code model types. However, you also said that this shouldn't be exposed.
Could you give me a bit more advice on how I should organise this? Thanks.
Just a heads up, but I think matklad is on vacation this week. I'm not sure about next.
I think this should be a method on https://github.com/rust-analyzer/rust-analyzer/blob/master/crates/ra_hir/src/code_model.rs#L1696-L1699
Implementing this for every variant of
Definition seems appropriate -- rust-analyzer is pretty boilerplate-heavy, and so far it seems like the strategy of writing a bit of boring code to get clearer boundaries between layers pays out
The saga of me not knowing where to put code continues...
I've moved quite a bit of the work into
ra_hir which is I think a reasonable place to put most of it. One function I'm struggling to place is
get_doc_url. It uses
ra_tt to parse the
html_doc_url attribute. I was thinking of moving it to a free function somewhere in
ra_hir_def, does that sound right?
I think it should be
In the future, I think we'll need some kind of way to flag "inert" attributes, so that these kind of features are implemenable outside of the base hir, but for now I think it's better to be simple and handle each attribute on the case-by-case basis.
Or, to be more precise, this function should retrieve the value of the attribute. Interpreting as an URL and selecting fallback should be done in the IDE layer I think
The problem is being a method on
Crate means it needs to be in
ra_hir, which doesn't import
ra_tt. So I think the actual implementation needs to be lower down, but again I have no idea where to put it. I contemplated
ra_hir_def::attr::AttrQuery but all of that code seems a lot lower level, and it seems wrong considering it'll only ever be defined on a root module.
I'm working on an old version of the codebase (
6b7cb8b5a), so things may have moved around a bit since.
ra_hir can import
ra_tt, it can't re-export it
Got it. I think I've resolved most of the issues on the PR now so should be ok for a second review when you have time :smile: .