Stream: t-compiler/rust-analyzer

Topic: Windows paths


ivan770 (Feb 10 2021 at 13:40, on Zulip):

Hi! Is there any pattern to determine if Windows drive letter has to be lower-cased or upper-cased? Kinda stuck with https://github.com/rust-analyzer/rust-analyzer/pull/7596, because of failing Windows build :upside_down:

ivan770 (Feb 10 2021 at 13:42, on Zulip):

I think main trouble for understanding is that https://github.com/rust-analyzer/rust-analyzer/pull/7596/checks?check_run_id=1871366255#step:9:2450 and https://github.com/rust-analyzer/rust-analyzer/pull/7596/checks?check_run_id=1871366255#step:9:2457 have different drive letters, even though they are in the same response.

matklad (Feb 10 2021 at 13:42, on Zulip):

Hm

matklad (Feb 10 2021 at 13:44, on Zulip):

I think this is more complicated that that, sadly. The problem is, there isn't an easy way to write tests at this layer of abstraction. But that should be OK, because this layer of abstraction should be relatively dumb.

matklad (Feb 10 2021 at 13:45, on Zulip):

That is, the problem is that the code we want to test lives in the place where testing is awkward. We can do one of the two:

ivan770 (Feb 10 2021 at 13:47, on Zulip):

I'm not sure if we really need to remove those tests, since they are testing Linux/Mac at the very least

matklad (Feb 10 2021 at 13:48, on Zulip):

That's the problem exactly: there's no reason why reference code lens need to have anything to do with mac vs linux.

matklad (Feb 10 2021 at 13:48, on Zulip):

the very fact that we observe the diff means that we are testing the wrong thing here.

ivan770 (Feb 10 2021 at 13:50, on Zulip):

Oh, got it

matklad (Feb 10 2021 at 13:52, on Zulip):

Roughly, everything there is in handle_code_lens should be a method of Analysis instead

matklad (Feb 10 2021 at 13:53, on Zulip):

and handle_ should just convert that to LSP, instead of containing actual logic, like

matches!(
                    it.kind,
                    SymbolKind::Trait | SymbolKind::Struct | SymbolKind::Enum | SymbolKind::Union
                )
matklad (Feb 10 2021 at 14:09, on Zulip):

So, I imagine we want API like the following in ide/lib.rs

pub struct Annotation {
    range: TextRange,
    annotation_kind: AnnotationKind
}
pub enum AnnotationKind {
    Runnable,
    HasImpls,
    HasReferences,
}

impl Analysis {
    pub fn annotations(&self, file_id: FileId) -> Cancelable<Annotation> {
    }

    pub fn resolve_annotation(&self, file_id: FileId) -> Cancelable<Option<ResolvedAnnotation>> {
    }
}
matklad (Feb 10 2021 at 14:10, on Zulip):

Than, handle_code_lens and handle_code_lens_resolve are implemented in terms of thes methonds of analysis.

ivan770 (Feb 10 2021 at 14:40, on Zulip):

Hmm. Would it be actually better, if we accepted Annotation in resolve_annotation, just as LSP sends resolve request per every unresolved CodeLens?

matklad (Feb 10 2021 at 14:43, on Zulip):

right, it should be resolve_annotation(&self, file_id: FileId, annotation: Annotation), that is typo

matklad (Feb 10 2021 at 14:46, on Zulip):

If you'd like to fix that, please read https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/architecture.md#crateside and https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/architecture.md#cratesrust-analyzer sections of architecture md, they describe the reasons why the API needs to be structured that way

ivan770 (Feb 10 2021 at 14:46, on Zulip):

Thanks! I'll take a look at it.

matklad (Feb 10 2021 at 14:47, on Zulip):

Sorry for not noticing the impending yak shave from the start!

ivan770 (Feb 12 2021 at 11:41, on Zulip):

Almost done. Howerer, I still have lots of questions about RA architecture (mostly about where to put a function or a struct). For example, I added an annotation function to to_proto crate, and it depends on those two functions https://github.com/rust-analyzer/rust-analyzer/blob/de046bf4572a75cf534a2342358a422b2f18d01c/crates/rust-analyzer/src/handlers.rs#L1533-L1547. Where should I put both of these functions, so they would be available for https://github.com/rust-analyzer/rust-analyzer/blob/de046bf4572a75cf534a2342358a422b2f18d01c/crates/rust-analyzer/src/handlers.rs#L1621 and other possible components?

ivan770 (Feb 12 2021 at 13:15, on Zulip):

Moving to ide part is ready: https://github.com/rust-analyzer/rust-analyzer/pull/7596

matklad (Feb 12 2021 at 16:18, on Zulip):

show_references_command and friends return an lsp_types::Command, so to_proto feels like an appropriate place

matklad (Feb 12 2021 at 16:19, on Zulip):

to_proto::command::show_references even.

matklad (Feb 12 2021 at 16:21, on Zulip):

So, the helpers which return titles may live there as well.

Last update: Jul 26 2021 at 14:00UTC