-
Notifications
You must be signed in to change notification settings - Fork 10
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
Simplify purescript-ds-create-imenu-index
with hashtable
#27
Conversation
…oh, and I've grepped for other usages of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified to work for me (TM).
The function has implemented a poor man's hash table by manually enlisting keys and then using `cl-case` on values. Just replace this with a hashtable and thus simplify the code considerably. This makes it faster as well because the `cl-case + setq` is O(n) whereas `puthash` is O(1). The output of the function was tested on Pandoc's `Writers/Powerpoint/Output.hs` file of 2841 lines and comparing the hashsum of the output before the changes and after. It didn't change. The commit is also basically a backport of a similar change on purescript-mode that has been forked form haskell-mode at some point: purescript-emacs/purescript-mode#27
purescript-ds-create-imenu-index
to hashtablespurescript-ds-create-imenu-index
with hashtable
The function has implemented a poor man's hash table by manually enlisting keys and then using `cl-case` on values. Just replace this with a hashtable and thus simplify the code considerably. This makes it faster as well because the `cl-case + setq` is O(n) whereas `puthash` is O(1). The output of the function was tested on Pandoc's `Writers/Powerpoint/Output.hs` file of 2841 lines and comparing the hashsum of the output before the changes and after. It didn't change. The commit is also basically a backport of a similar change on purescript-mode that has been forked form haskell-mode at some point: purescript-emacs/purescript-mode#27
The function has implemented a poor man's hash table by manually enlisting keys and then using `cl-case` on values. Just replace this with a hashtable and thus simplify the code considerably. This makes it faster as well because the `cl-case + setq` is O(n) whereas `puthash` is O(1). The output of the function was tested on Pandoc's `Writers/Powerpoint/Output.hs` file of 2841 lines and comparing the hashsum of the output before the changes and after. It didn't change. The commit is also basically a backport of a similar change on purescript-mode that has been forked form haskell-mode at some point: purescript-emacs/purescript-mode#27
06286b0
to
60b4a73
Compare
UPD: changed for compatibility with older Emacs versions per discussion here |
Well, and now it fails to compile on snapshot due to "obsolete macro"… |
The function has implemented a poor man's hash table by manually enlisting keys and then jumping through the hoops by fetching them from one place, converting values to symbols to values… In particular, it was using `symbol-value` to map a symbol to its value, however the symbol was a local variable, and `symbol-value` doesn't work with them under lexical-binding, which the file was converted to since commit 9a9f550. So fix the regression and simplify the code at the same time. Fixes: purescript-emacs#25
60b4a73
to
e42b147
Compare
Okay, I added a wrapper macro to silence the warning, it builds now on snapshot without warnings. |
💪 thanks, merged. |
The function has implemented a poor man's hash table by manually enlisting keys and then jumping through the hoops by fetching them from one place, converting values to symbols to values… In particular, it was using
symbol-value
to map a symbol to its value, however the symbol was a local variable, andsymbol-value
doesn't work with them under lexical-binding, which the file was converted to since commit 9a9f550.So fix the regression and simplify the code at the same time.
Fixes: #25