Skip to content

Conversation

@gerau
Copy link
Collaborator

@gerau gerau commented Oct 20, 2025

No description provided.

gerau added 2 commits October 17, 2025 12:03
built-in `Span` of the `simplicityhl::parse::Call` calculated as full
call, with parameters and generics, so we use custom calculation of call
using length of the representing string
@KyrylR
Copy link
Collaborator

KyrylR commented Oct 22, 2025

Add gifs and list of features supported by the LSP to README pls

@KyrylR
Copy link
Collaborator

KyrylR commented Oct 22, 2025

Fix: Cmd + hover on the function shows funcs sig twice

src/backend.rs Outdated
let document = documents.get(uri)?;

let token_position = params.text_document_position_params.position;
let token_span = positions_to_span((token_position, token_position)).ok()?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
let token_span = positions_to_span((token_position, token_position)).ok()?;
let token_span = positions_to_span((token_position, token_position))?;

Add support for:
Conversion of tower_lsp_server::lsp_types::Position to simplicityhl::error::Span

src/backend.rs Outdated
.iter()
.find(|function| function.name() == func)?;

let (start, end) = span_to_positions(function.as_ref()).ok()?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same

src/backend.rs Outdated
.last()
}

fn get_call_span(call: &simplicityhl::parse::Call) -> Option<simplicityhl::error::Span> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use Result instead of Option pls

@KyrylR
Copy link
Collaborator

KyrylR commented Oct 22, 2025

Is it possible to use HashMap instead of document.functions.iter().find?

gerau and others added 7 commits October 22, 2025 18:00
change to match behaviour of `hover` command, which is reduces
boilerplate with `Result`, because we don't throw errors on this
functions
- moved unrelated function to utils.rs
- add several small unittests
it combines function and docs into one HashMap with clear interface for
getting functions or docs for them
@gerau gerau marked this pull request as ready for review October 24, 2025 11:15
@gerau
Copy link
Collaborator Author

gerau commented Oct 24, 2025

@KyrylR

gerau and others added 4 commits October 24, 2025 14:30
methods in `utils.rs` was returning different kind of error:
`TryFromIntError` and `jsonrpc::Error` which was difficult work with.
`LspError` support converting from `TryFromIntError` and to
`jsonrpc::Error`
@KyrylR
Copy link
Collaborator

KyrylR commented Oct 28, 2025

Use thiserror pls

README.md Outdated
- Basic diagnostic for SimplicityHL code
- Completions of functions and jets

![diagnostics](https://github.com/user-attachments/assets/135ba63b-d05b-48dd-8047-7ca904494f0b)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not use user attachments

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add gifs directly into the repo


/// Get generic functions completions.
pub fn get_function_completions(functions: &[(Function, String)]) -> Vec<CompletionItem> {
pub fn get_function_completions(functions: &Vec<(&Function, &String)>) -> Vec<CompletionItem> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

src/utils.rs Outdated
}
})
.filter(|(_, span)| span_contains(span, &token_span))
.map(|(call, _)| call.clone())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to avoid cloning?

Copy link
Collaborator

@KyrylR KyrylR left a comment

Choose a reason for hiding this comment

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

LGTM

@KyrylR KyrylR merged commit ca87c10 into main Oct 31, 2025
1 check passed
@gerau gerau deleted the dev branch October 31, 2025 13:02
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.

3 participants