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

Lazy static completions #20

Closed
wants to merge 8 commits into from
Closed

Lazy static completions #20

wants to merge 8 commits into from

Conversation

lavafroth
Copy link
Contributor

@lavafroth lavafroth commented Sep 14, 2023

Changes

  • Replace HX_TAGS, HX_ATTRIBUTE_VALUES and TEXT_STORE from being OnceLocks to being lazy statics
  • init_text_store and init_hx_tags are now implicit
  • Lazy statics no longer require .get() for accessing attributes

@ThePrimeagen
Copy link
Owner

could we break these up into 2 prs?

please do the lazy statics. i like that change a lot :)

then we can avoid the clones

@lavafroth
Copy link
Contributor Author

The point about lesser clones is a by-product of using lazy statics. The lazy static forces the use of static string slices in the cases I mentioned unless one goes out of their way to clone things. 😅

@lavafroth
Copy link
Contributor Author

lavafroth commented Sep 26, 2023

Also, it would be better in the long run to have the binary representation of the completion structs embedded in the final executable during comptime so we don't have to call .leak() on the completions vector. We could use the include! macro to do something like that.

@ThePrimeagen
Copy link
Owner

ok, very interesting. i am going to try to review this once more and make sure i get what is going on. :)

i may just need to download it and look at it in my editor

@ThePrimeagen
Copy link
Owner

i am also going to get #22 merged in first.

may cause conflict, sorry

@lavafroth
Copy link
Contributor Author

Sure thing! I've synced this branch with upstream.

@ThePrimeagen
Copy link
Owner

psst, sorry about not merging this

if you remove the conflicts, i can help get this in

@lavafroth
Copy link
Contributor Author

lavafroth commented Oct 11, 2023 via email

@lavafroth
Copy link
Contributor Author

I'm only considering the inclusion of lazy static for now, if I try to add more changes this PR becomes really massive.

@lavafroth
Copy link
Contributor Author

Hey just a heads up, it looks like OnceCell / OnceLock are actually more mature for our use case. There's also talk about deprecating lazy_static. Sorry for troubling you, I'm closing this.

@lavafroth lavafroth closed this Oct 12, 2023
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.

2 participants