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

[WIP] Update LSP hover action to return labelled definition #2781

Closed
wants to merge 3 commits into from

Conversation

manuraj17
Copy link

Relates to #2653

Updating the LSP hover action to return the full function head, as per the documentation.

The planned format is as below

<fn-accessor-type> fn <function-name>(<label> <argument-name>: <type>, ..) -> <return-type> 

Eg:
For a function like below

fn greet(firstname name:String) { io.println("Hello " <> name) }

The generated string would be

fn greet(firstname name: String) -> Nil 

Current implementation uses pretty print and existing Fn Type enum. Since labels are not part of the type system, we are going ahead with simple string interpolation for now.

P.S : This is the first time working with the codebase and such a big Rust repo, hence please feel free to suggest changes and improvements.

P.P.S: I am trying to test it out in local hence kept the [WIP] tag.

LSP hover action was returning the function type signatures.
It is now modified to return the full explicit function head
with name and labelled arguments along with return type
};
let formatted_type = Printer::new().pretty_print(&function_type, 0);

let mut function_string = format!("fn {}(", fun.name);
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW https://github.com/gleam-lang/gleam/pull/2705/files it looks like there is an existing draft pr that utilizes the function used by the doc generation to do roughly the same thing https://github.com/gleam-lang/gleam/blob/main/compiler-core/src/format.rs#L1403 not sure if theres a preference to reuse vs write new here but worth considering

Copy link
Author

Choose a reason for hiding this comment

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

@Acepie Oh nice, I think it's targeting same set of changes. I wasn't aware of this format. Looks like that way is better. I will discuss on that PR 👍🏽 Thanks.

@manuraj17
Copy link
Author

Considering the sampe like below

import gleam/io
import gleam/string

fn greet(n name: String) -> String {
  string.append(name, "hello")
}

pub fn main() {
  io.println(greet("Tom"))
}

Some of my findings (being new to the language)

  • Hovering on main() our Lsp identifies the keyword as a function and it works fine
  • Hovering on greet on this line io.println(greet("Tom")) , it get idenfitied as expression here

Will need to identify how to handle that case there. Plus, I am also trying to understand how to quickly identify if the expression in this method here

Tried defining something like below but seems to not to work

    /// Returns `true` if the typed expr is [`Fn`].
    ///
    /// [`Fn`]: TypedExpr::Fn
    #[must_use]
    pub fn is_function(&self) -> bool {
        matches!(self, Self::Fn { .. })
    }

cc @lpil

@Acepie
Copy link
Contributor

Acepie commented Mar 30, 2024

The case you are probably looking for there is a Self::Call not a Self::Fn fn is a function definition whereas call is when the function is actually being used

@lpil
Copy link
Member

lpil commented May 20, 2024

Closing due to inactivity. Please feel free to reopen! Thank you

@lpil lpil closed this May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants