LS: Introduce hovers for literals

[integraledelebesgue]

Introduced changes:

• LS now displays hovers with descriptions above literal expressions
• Descriptions contain information about the type and possible values in available representations

---

This change is 

[mkaput]

Reviewed 6 of 9 files at r1, all commit messages.
Reviewable status: 6 of 9 files reviewed, 4 unresolved discussions (waiting on @integraledelebesgue)

---

crates/cairo-lang-language-server/src/ide/hover/mod.rs line 22 at r1 (raw file):

    let position = params.text_document_position_params.position.to_cairo();

    db.find_hover_target_at_position(file_id, position)?.render(db, file_id)

ok, this is basically the exact opposite of what renderers were meant for :D

each renderer's responsibility is to find its exact target inside and return None if they had no target. by chaining renders, we formed a chain of fallbacks. this allows treating each hover kind as some kind of a black box, each renderer being completely separate from each other.

that should happen here is that:

1. you try to show a hover for a literal, it would quickly return None if hovering over non-literal terminal
2. then try showing a definition of something
3. finally try legacy hovers which we want to eradicate

---

crates/cairo-lang-language-server/src/ide/hover/range.rs line 18 at r1 (raw file):

            .span_without_trivia(db.upcast())
            .position_in_file(db.upcast(), file_id)
            .map(|p| p.to_lsp())

TBH this isn't a chain of method calls that is worth deduplicating. It is long, but clearly leads to the result and has no complex logic.

---

crates/cairo-lang-language-server/src/ide/hover/render/legacy.rs line 95 at r1 (raw file):

            s
        }
        other => format!("Type: {}", other.ty().format(db)),

please do not touch legacy hovers

---

crates/cairo-lang-language-server/src/ide/hover/render/mod.rs line 3 at r1 (raw file):

pub(super) use self::definition::*;
pub(super) use self::legacy::*;
pub(super) use self::literal::*;

all these (super) are unnecessary, the bring no value

[integraledelebesgue]

Reviewable status: 2 of 10 files reviewed, 4 unresolved discussions (waiting on @mkaput)

---

crates/cairo-lang-language-server/src/ide/hover/mod.rs line 22 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

ok, this is basically the exact opposite of what renderers were meant for :D

each renderer's responsibility is to find its exact target inside and return None if they had no target. by chaining renders, we formed a chain of fallbacks. this allows treating each hover kind as some kind of a black box, each renderer being completely separate from each other.

that should happen here is that:

1. you try to show a hover for a literal, it would quickly return None if hovering over non-literal terminal
2. then try showing a definition of something
3. finally try legacy hovers which we want to eradicate

I changed the implementation to do exactly what you described :)

---

crates/cairo-lang-language-server/src/ide/hover/render/legacy.rs line 95 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

please do not touch legacy hovers

Sure, I restored the old snippet and also added one branch that makes the legacy rendered ignore literals.

[integraledelebesgue]

Reviewable status: 1 of 10 files reviewed, 4 unresolved discussions (waiting on @mkaput)

---

crates/cairo-lang-language-server/src/ide/hover/render/mod.rs line 3 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

all these (super) are unnecessary, the bring no value

Done.

[piotmag769]

Reviewed 1 of 9 files at r1, 6 of 8 files at r2, 1 of 2 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, @mkaput, and @orizi)

---

crates/cairo-lang-language-server/src/ide/hover/mod.rs line 21 at r4 (raw file):

    render::definition(db, &identifier, file_id).or_else(|| render::legacy(db, &identifier))

    // TODO(mkaput): If client only supports plaintext, strip markdown formatting here like RA.

Why did we remove that?

---

crates/cairo-lang-language-server/src/ide/hover/range.rs line 18 at r1 (raw file):

Previously, mkaput (Marek Kaput) wrote…

TBH this isn't a chain of method calls that is worth deduplicating. It is long, but clearly leads to the result and has no complex logic.

Agree, overengineering here

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 43 at r4 (raw file):

    let number_type = type_suffix.as_ref().map(SmolStr::as_str).unwrap_or("felt252");
    let type_path = if number_type == "felt252" { "core" } else { "core::integer" };

You should get the type from semantic resolver (you would have to travel deeper like in e.g. missing_traits_actions), this will result in wrong result e.g. for

let _: u8 = 60;

alternatively you can just drop the type if it is too much work since it will get pretty complex, wdyt @mkaput?

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 93 at r4 (raw file):

            {TITLE}: `'{string}' ({numeric:#x})`
            {RULER}
            Type: `core::felt252`

I can do sth like let _: u8 = 'a'; and it will work, you cannot assume a type this way here

[mkaput]

Reviewed 6 of 8 files at r2, 1 of 2 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, @orizi, and @piotmag769)

a discussion (no related file):
please cover this feature thoroughly in e2e test. don't forget about unhappy cases!

---

---

crates/cairo-lang-language-server/src/ide/hover/mod.rs line 21 at r4 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

Why did we remove that?

yeah, this comment is still relevant

---

crates/cairo-lang-language-server/src/ide/hover/mod.rs line 22 at r4 (raw file):

    if let Some(hover) =
        db.find_literal_at_position(file_id, position).map(|literal| literal.render(db, file_id))

use find_syntax_node_at_position here and narrow it down to literal within render::literal function

---

crates/cairo-lang-language-server/src/ide/hover/render/definition.rs line 9 at r4 (raw file):

use crate::ide::hover::markdown_contents;
use crate::ide::hover::range::HoverRange;
use crate::ide::hover::render::markdown::{RULE, fenced_code_block};

run scripts/rust_fmt.sh. you may need to merge latest main changes

---

crates/cairo-lang-language-server/src/ide/hover/render/legacy.rs line 95 at r1 (raw file):

Previously, integraledelebesgue (Jan Smółka) wrote…

Sure, I restored the old snippet and also added one branch that makes the legacy rendered ignore literals.

no need for doing this given that you catch literals before even attempting to try rendering legacy, isn't it?

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 24 at r4 (raw file):

impl Literal {
    /// Generate a hover containing value of the literal and its type.
    pub fn render(&self, db: &AnalysisDatabase, file_id: FileId) -> Option<Hover> {

please make this a free-standing function as other render ones are written. let's keep code uniform 😃

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 43 at r4 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

You should get the type from semantic resolver (you would have to travel deeper like in e.g. missing_traits_actions), this will result in wrong result e.g. for

let _: u8 = 60;

alternatively you can just drop the type if it is too much work since it will get pretty complex, wdyt @mkaput?

I think the only correct path here is to use type information from the semantic model. Any heuristic attempts will be doomed to fail + we would have to spend time keeping them up to date with any type inference changes that may happen in the future.

---

crates/cairo-lang-language-server/src/lang/db/syntax.rs line 62 at r4 (raw file):

    /// [`TerminalLiteralNumber`], [`TerminalString`] or [`TerminalShortString`]
    /// at the given [`TextPosition`] in file.
    fn find_literal_at_position(&self, file: FileId, position: TextPosition) -> Option<Literal> {

in general we have an unspoken rule that no ide stuff should be used from lang side -- this always means that things are wrongly architected because we have a backward dependency in code

the code you're doing here is 100% fine to be kept within ide::hover::literal module, as private logic

---

crates/cairo-lang-language-server/src/lang/inspect/defs.rs line 23 at r4 (raw file):

use cairo_lang_syntax::node::kind::SyntaxKind;
use cairo_lang_syntax::node::utils::is_grandparent_of_kind;
use cairo_lang_syntax::node::{ast, SyntaxNode, Terminal, TypedStablePtr, TypedSyntaxNode};

old formatting

[mkaput]

Reviewed 6 of 8 files at r5, all commit messages.
Reviewable status: 8 of 10 files reviewed, 7 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, @orizi, and @piotmag769)

[piotmag769]

Reviewed 8 of 8 files at r5, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, and @orizi)

[mkaput]

Reviewed 1 of 8 files at r5.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 17 at r5 (raw file):

const RULER: &str = "***";

/// Narrow down [`SyntaxNode`] to [`TerminalLiteralNumber`], [`TermialString`] or

that's the form we should use in docs. see how rust's std is documented

Suggestion:

/// Narrows

[mkaput]

Reviewed 1 of 8 files at r5.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 15 at r5 (raw file):

const TITLE: &str = "value of literal";
const RULER: &str = "***";

I would personally inline both of these constants because they make “templates” less readable to me. wdyt?

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 56 at r5 (raw file):

        {TITLE}: `{value} ({value:#x} | {value:#b})`
        {RULER}
        Type: `{type_path}::{number_type}`

I don't like this Type: part, let's just put it as-is first, like RA:

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 57 at r5 (raw file):

        {RULER}
        Type: `{type_path}::{number_type}`
        "

can you add all the nice conversions that https://www.stark-utils.xyz/converter does?

[integraledelebesgue]

Reviewable status: 9 of 10 files reviewed, 8 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/mod.rs line 21 at r4 (raw file):

Previously, mkaput (Marek Kaput) wrote…

yeah, this comment is still relevant

I removed it by accident 💀

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 43 at r4 (raw file):

Previously, mkaput (Marek Kaput) wrote…

I think the only correct path here is to use type information from the semantic model. Any heuristic attempts will be doomed to fail + we would have to spend time keeping them up to date with any type inference changes that may happen in the future.

I implemented a new type acquisition logic. It works fine when applied to literals in a function context. However, my method doesn't apply to const definitons because they have no associated FunctionWithBodyId.

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 93 at r4 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

I can do sth like let _: u8 = 'a'; and it will work, you cannot assume a type this way here

Done.

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 15 at r5 (raw file):

Previously, mkaput (Marek Kaput) wrote…

I would personally inline both of these constants because they make “templates” less readable to me. wdyt?

Sure, not much gain from making them constant :)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 56 at r5 (raw file):

Previously, mkaput (Marek Kaput) wrote…

I don't like this Type: part, let's just put it as-is first, like RA:

Done.

---

crates/cairo-lang-language-server/src/lang/inspect/defs.rs line 23 at r4 (raw file):

Previously, mkaput (Marek Kaput) wrote…

old formatting

Done.

[integraledelebesgue]

Reviewable status: 9 of 10 files reviewed, 8 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 17 at r5 (raw file):

Previously, mkaput (Marek Kaput) wrote…

that's the form we should use in docs. see how rust's std is documented

Sure, I changed it. Both hover::mod and hover::render::definition have docstrings in an imperative form. Should I correct them too?

[mkaput]

Reviewed 1 of 1 files at r7, all commit messages.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 43 at r4 (raw file):

Previously, integraledelebesgue (Jan Smółka) wrote…

I implemented a new type acquisition logic. It works fine when applied to literals in a function context. However, my method doesn't apply to const definitons because they have no associated FunctionWithBodyId.

But I see you've fixed that obstacle anyway, haven't you? :)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 17 at r5 (raw file):

Previously, integraledelebesgue (Jan Smółka) wrote…

Sure, I changed it. Both hover::mod and hover::render::definition have docstrings in an imperative form. Should I correct them too?

the rule of thumb is don't touch the code that you don't have to 🙃

yeah we all tend to mix these forms everywhere, and it sometimes slip review :sad:

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 69 at r7 (raw file):

}

// TODO: think about something better

?

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 116 at r7 (raw file):

    db: &AnalysisDatabase,
    literal: &TerminalString,
    r#type: &str,

that's the convention we have everywhere, so it's fine name

Suggestion:

ty

[integraledelebesgue]

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 43 at r4 (raw file):

Previously, mkaput (Marek Kaput) wrote…

But I see you've fixed that obstacle anyway, haven't you? :)

A solution the TODO you noticed refers to works, but not exactly as I would like. I described the problem in that discussion.

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 69 at r7 (raw file):

Previously, mkaput (Marek Kaput) wrote…

?

The problem with that solution is that it returns only a type name without the full path. That's because how SyntaxNode::get_text works. I can't use the semantic analysis machinery I utilise in find_type_in_function_context because, as I understand, it works only in function context :(
That's why I left TODO :))

[orizi]

Reviewed all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, @mkaput, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 80 at r7 (raw file):

    let declaration = ItemConstant::from_syntax_node(db, node);
    let r#type = declaration.type_clause(db).ty(db).as_syntax_node().get_text(db);
    Some(r#type)

Suggestion:

    Some(declaration.type_clause(db).ty(db).as_syntax_node().get_text(db))

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 116 at r7 (raw file):

Previously, mkaput (Marek Kaput) wrote…

that's the convention we have everywhere, so it's fine name

agreed - and everywhere else in this PR.

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 143 at r7 (raw file):

/// Formats the short string literal writing its textual and numeric value along with the
/// `core::felt252` type.
fn short_string(

i think this would be better (for the other 2 functions as well) - as I find the lack of context in the call itself confusing.

Suggestion:

fn short_string_hover(

[integraledelebesgue]

Reviewable status: 8 of 11 files reviewed, 8 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 80 at r7 (raw file):

    let declaration = ItemConstant::from_syntax_node(db, node);
    let r#type = declaration.type_clause(db).ty(db).as_syntax_node().get_text(db);
    Some(r#type)

Done.

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 116 at r7 (raw file):

Previously, orizi wrote…

agreed - and everywhere else in this PR.

Done.

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 143 at r7 (raw file):

Previously, orizi wrote…

i think this would be better (for the other 2 functions as well) - as I find the lack of context in the call itself confusing.

Indeed, the call itself does not carry the whole context. However, it's easily discoverable via return type, so I think it's no use duplicating that informaton in the name.

[orizi]

Reviewed all commit messages.
Reviewable status: 8 of 11 files reviewed, 7 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, @mkaput, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 143 at r7 (raw file):

Previously, integraledelebesgue (Jan Smółka) wrote…

Indeed, the call itself does not carry the whole context. However, it's easily discoverable via return type, so I think it's no use duplicating that informaton in the name.

the return type does not appear in the callsite - my issue it is that the callsite of this function seems very weird - it sounds like construction - but it is a very specific function call.
Hover::short_string would have been ok - but this seems unclear to me.

[integraledelebesgue]

Reviewable status: 8 of 11 files reviewed, 7 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 143 at r7 (raw file):

Previously, orizi wrote…

the return type does not appear in the callsite - my issue it is that the callsite of this function seems very weird - it sounds like construction - but it is a very specific function call.
Hover::short_string would have been ok - but this seems unclear to me.

Sure, I see your point. I changed the names. I've only kept the top level function name - render::literal - the same to keep it consistent with already existing renderers.

[integraledelebesgue]

Reviewable status: 8 of 11 files reviewed, 7 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)

a discussion (no related file):

Previously, mkaput (Marek Kaput) wrote…

please cover this feature thoroughly in e2e test. don't forget about unhappy cases!

I added some tests for various scenarios. Please have a look at them. I don't know whether I can assert some error messages in them. In my implementation, hovers appear even when e.g. types are mismatched (see the example here).

---

[orizi]

Reviewed 1 of 3 files at r8, all commit messages.
Reviewable status: 9 of 11 files reviewed, 6 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, @mkaput, and @piotmag769)

[piotmag769]

Reviewed 1 of 3 files at r8, 2 of 2 files at r9, all commit messages.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, and @mkaput)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 69 at r7 (raw file):

Previously, integraledelebesgue (Jan Smółka) wrote…

The problem with that solution is that it returns only a type name without the full path. That's because how SyntaxNode::get_text works. I can't use the semantic analysis machinery I utilise in find_type_in_function_context because, as I understand, it works only in function context :(
That's why I left TODO :))

Create an issue and mark it with issue number pls

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 43 at r9 (raw file):

            short_string_hover(db, &literal, &ty, file_id)
        }
        _ => None,

It may be out of scope, but what about fixed size arrays? let x: [u8; 2] = [1, 2] cc @mkaput

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 111 at r9 (raw file):

/// Formats the number literal writing it along with the `core::byte_array::ByteArray` type.
fn string_hover(

Should we support that? Are there plans for byte array consts? cc @orizi

---

crates/cairo-lang-language-server/tests/test_data/hover/basic.txt line 695 at r9 (raw file):

// = popover
```cairo
core::integer::u8

This doesn't work, there are not ByteArray literals rn

[mkaput]

Reviewed 1 of 3 files at r8, 2 of 2 files at r9, all commit messages.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, and @piotmag769)

a discussion (no related file):

Previously, integraledelebesgue (Jan Smółka) wrote…

I added some tests for various scenarios. Please have a look at them. I don't know whether I can assert some error messages in them. In my implementation, hovers appear even when e.g. types are mismatched (see the example here).

We're only interested in hovers in these tests, so it is very very fine not to assert on diagnostics :)

---

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 69 at r7 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

Create an issue and mark it with issue number pls

I would rather fix this here. Shipping incorrect & misleading logic is worse than not shipping it at all.

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 43 at r9 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

It may be out of scope, but what about fixed size arrays? let x: [u8; 2] = [1, 2] cc @mkaput

1. out of scope, this is not a literal
2. what would you like to see in these hovers?

anyway, make a separate task for these if you wish :)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 95 at r9 (raw file):

        {ty}
        ```
        ***

shouldn't this be dashes?

Suggestion:

---

---

crates/cairo-lang-language-server/tests/test_data/hover/basic.txt line 81 at r9 (raw file):

}

const SOME_CONST: felt252 = 0x<caret>2137;

please move these tests to a new fixture, say literals.txt

we don't want to grow fixtures horizontally because they quickly become unreadable

---

crates/cairo-lang-language-server/tests/test_data/hover/basic.txt line 680 at r9 (raw file):

    let a: u32 = 0x2<caret>137_felt252;
// = highlight
    let a: u32 = <sel>0x2137_felt252</sel>;

this is code, do not use memes (esp. controversial) here

[integraledelebesgue]

Reviewable status: 8 of 13 files reviewed, 7 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/tests/test_data/hover/basic.txt line 81 at r9 (raw file):

Previously, mkaput (Marek Kaput) wrote…

please move these tests to a new fixture, say literals.txt

we don't want to grow fixtures horizontally because they quickly become unreadable

Done.

[integraledelebesgue]

Reviewable status: 11 of 13 files reviewed, 3 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 69 at r7 (raw file):

Previously, mkaput (Marek Kaput) wrote…

I would rather fix this here. Shipping incorrect & misleading logic is worse than not shipping it at all.

We improved this logic. Please have a look @mkaput :)

[piotmag769]

Reviewed 2 of 2 files at r11, 1 of 1 files at r12, all commit messages.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, and @mkaput)

a discussion (no related file):
Please add tests for consts

---

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 111 at r9 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

Should we support that? Are there plans for byte array consts? cc @orizi

Still stand by the point that we shouldn't display anything there cc @mkaput

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 78 at r12 (raw file):

    while node.kind(db) != SyntaxKind::ItemConstant {
        node = node.parent()?;
    }

Suggestion:

    let const_node = db.first_ancestor_of_kind(node, SyntaxKinda::ItemConstant);

[mkaput]

Reviewed 2 of 2 files at r11, 1 of 1 files at r12, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, and @orizi)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 69 at r7 (raw file):

Previously, integraledelebesgue (Jan Smółka) wrote…

We improved this logic. Please have a look @mkaput :)

awesome

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 111 at r9 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

Still stand by the point that we shouldn't display anything there cc @mkaput

I think we can simply show the type? Indeed, the value of the literal is not informative here as I think now 🤔

[integraledelebesgue]

Reviewable status: 11 of 13 files reviewed, 4 unresolved discussions (waiting on @Arcticae, @Draggu, @mkaput, @orizi, and @piotmag769)

a discussion (no related file):

Previously, piotmag769 (Piotr Magiera) wrote…

Please add tests for consts

I have one happy and one unhappy case at the moment. Please let me know if this is enough

---

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 43 at r9 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

what would you like to see in these hovers?

Actually no idea, I would check what RA does here ;p would leave it for now though

RA simply shows values of particular elements, as my implementation does at the moment

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 111 at r9 (raw file):

Previously, mkaput (Marek Kaput) wrote…

I think we can simply show the type? Indeed, the value of the literal is not informative here as I think now 🤔

Sure, I show only types now. However, in case of mismatch:

let x: u8 = "0";

hover shows the declared type - u8.
I don't know whether it's very problematic. Please let me know @mkaput @piotmag769

[piotmag769]

Reviewed 2 of 2 files at r13, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Arcticae, @Draggu, @integraledelebesgue, and @orizi)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 111 at r9 (raw file):

Previously, integraledelebesgue (Jan Smółka) wrote…

Sure, I show only types now. However, in case of mismatch:

let x: u8 = "0";

hover shows the declared type - u8.
I don't know whether it's very problematic. Please let me know @mkaput @piotmag769

It's fine for me. Literals don't have types by themselves

[integraledelebesgue]

Reviewable status: 12 of 13 files reviewed, 4 unresolved discussions (waiting on @Arcticae, @Draggu, @orizi, and @piotmag769)

---

crates/cairo-lang-language-server/src/ide/hover/render/literal.rs line 78 at r12 (raw file):

    while node.kind(db) != SyntaxKind::ItemConstant {
        node = node.parent()?;
    }

Done.

---

crates/cairo-lang-language-server/tests/test_data/hover/basic.txt line 695 at r9 (raw file):

Previously, piotmag769 (Piotr Magiera) wrote…

This doesn't work, there are not ByteArray literals rn

Is it still relevant since we only display types for string literals?

[piotmag769]

Reviewed 1 of 1 files at r14, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @Arcticae, @Draggu, and @orizi)

[piotmag769]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Arcticae, @Draggu, and @orizi)

[mkaput]

Reviewed 1 of 2 files at r13, 1 of 1 files at r14, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Arcticae, @Draggu, and @orizi)

[orizi]

Reviewed 1 of 8 files at r2, 6 of 8 files at r5, 1 of 5 files at r10, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Arcticae and @Draggu)