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

Cleanups for clippy and text_store #29

Merged

Conversation

vihu
Copy link
Contributor

@vihu vihu commented Oct 16, 2023

Summary

  • The most notable change here is the removal of unnecessary clones as suggested by clippy. Furthermore, this also makes the TextStore a wrapper type of a HashMap<String, String> and implements Deref and DerefMut for it.

  • Workflow update to ensure that clippy runs in more exhaustive way (this is actually how I found about the unnecessary cloning).

Summary
----
- The most notable change here is the removal of unnecessary clones as
suggested by clippy. Furthermore, this also makes the TextStore a
wrapper type of a `HashMap<String, String>` and implements Deref and
DerefMut for it.

- Workflow update to ensure that clippy runs in _more_ exhaustive way
  (this is actually how I found about the unnecessary cloning).
@MNThomson
Copy link
Collaborator

LGTM for adding the clippy lint for GH Actions. Needs @ThePrimeagen's review for LSP changes

}

pub fn get_text_document(uri: Url) -> Option<String> {
return TEXT_STORE
TEXT_STORE
Copy link

Choose a reason for hiding this comment

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

please use explicit return !

Copy link
Owner

Choose a reason for hiding this comment

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

yeah this isn't going to fly for me fyi

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in ba4e324

@ThePrimeagen
Copy link
Owner

i'll be glad to take this pr once explicit returns are returned :)

@ThePrimeagen ThePrimeagen merged commit 1f4071f into ThePrimeagen:master Dec 3, 2023
3 checks passed
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.

4 participants