HLS Plugin to provide Alternate Literal Formats. #2350
Conversation
Update our parsing module to return the data type we will use in our plugin. The datatype: Literal Constructors: Overloaded - Contains an overloaded literal with a SrcSpan NonOverloaded - Contains non-overloaded literals with a SrcSpan NoLocation - currently unused. This data type is bound to change. This module is designed to capture ALL numeric literals for use in the plugin. When parsing Haskell source tree, certain cases result in getting a plain `Expr`. This means it is not annotated with a SrcSpan. As a result, we pass along a SrcSpan through most functions to keep track of the most recent SrcSpan encountered. This isn't a fool proof plan and may be removed in the future.
The traversal of Haskell source has been turned into a rule. This rule generates hidden diagnostics which allow a user to run a code action over (most) literals. There is some "dead code" in the sense that some literals (prims to be exact) are not allowed to be changed, despite existing in code. The other "dead code" exists in the FormatType type. Certain extensions exist that alter how numbers are shown or act, we make note of those extensions but call them "NoFormat", which essentially means it's a NOOP. Most of the conversion is done via the Numeric module. There are plenty of "show" functions that provide the translation of numbers into the various formats. The only custom format is a slightly "dumb" NumDecimal-extension-function that generates three separate options, instead of using a numeric defined function.
…server into alternate-number-format
…server into alternate-number-format
plugins/hls-alternate-number-format-plugin/hls-alternate-number-format-plugin.cabal
Outdated
Show resolved
Hide resolved
plugins/hls-alternate-number-format-plugin/src/Ide/Plugin/Conversion.hs
Outdated
Show resolved
Hide resolved
| -- Generate up to 3 possible choices where: | ||
| -- dropWhile (\d -> val `div` d) > 1000) implies we want at MOST 3 digits to left of decimal |
There was a problem hiding this comment.
| -- Generate up to 3 possible choices where: | |
| -- dropWhile (\d -> val `div` d) > 1000) implies we want at MOST 3 digits to left of decimal | |
| -- | Generate up to 3 possible choices where: | |
| -- dropWhile (\d -> val `div` d) > 1000) implies we want at MOST 3 digits to left of decimal |
There was a problem hiding this comment.
This way it is haddock compliant and on hover you actually see the comment.
Additionally, check for valid Haddock syntax so it renders properly.
plugins/hls-alternate-number-format-plugin/src/Ide/Plugin/Conversion.hs
Outdated
Show resolved
Hide resolved
plugins/hls-alternate-number-format-plugin/src/Ide/Plugin/Conversion.hs
Outdated
Show resolved
Hide resolved
plugins/hls-alternate-number-format-plugin/src/Ide/Plugin/AlternateNumberFormat.hs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Awesome, this is a great plugin! I have some general comments and ideas but feel free to disagree.
Traversing the whole AST for each change might be very expensive, so we should see whether we can optimise that somehow.
I've looked into your testsuite issue, but can't tell right now what the issue is.
|
Many thanks for working on this! It looks great overall
A usual way is use It has bounds on base, ghcide and hls-plugin-api and nothing more
If you have the chance of building your version for other ghc versions (f.e 8.6.5, 8.8.4 and 9.0.1) it would be great. But We have the ci covering all supported versions so you can leverage it. I would add a line like haskell-language-server/.github/workflows/test.yml Lines 187 to 189 in ab46fe8
Well, a good way to trace test problems is enable full log output of lsp messages, take a look to the test suite linked just above and use the same invocation locally (the relevant part is |
Clean up function names and add supporting haddock documentation for exported functions.
I ran the tests with logs enabled. Looks like an exception occurs (which I think is relevant?) and then a shutdown message is sent to the server. It looks like it's attempting to do some actual work but for whatever reason a shutdown message is sent?
|
|
@drsooch i have taken the liberty of add the plugin to the rest of cabal.project's (for ghc 9.0.1 and 9.2.1) and to the test workflow, to see how is going in ci |
That message about finishing the build is normal. I guess the test is not finding the code action, not sure why Take a look to this test in the haddock plugin: It is looking for a code action in a specific code range, maybe you need something similar to trigger it? |
|
Hi, it seems the plugin is not buildable with ghc-8.6.5, a version we still support. I hope only changing plugin base constraints could make the build work: https://github.com/haskell/haskell-language-server/runs/4209830202?check_suite_focus=true |
Yeah, I started testing out 9.0.1 as well, and there are some changes needed However, those are pending @fendor's comments about using syb. I will aim to clean up the builds for each GHC version tonight. |
plugins/hls-alternate-number-format-plugin/hls-alternate-number-format-plugin.cabal
Outdated
Show resolved
Hide resolved
…language-server into alternate-number-format
What about |
oh, you are damn right, thanks for noting it |
|
There is a failing test plugin related for ubuntu and 8.10.7: |
done with eb05482 |
…language-server into alternate-number-format
|
/rerun-workflow Testing |
* Initialize and Setup Plugin Source. * Provide Traversal Module to Haskell AST that collects Literals. * Change Literals module to return type. Update our parsing module to return the data type we will use in our plugin. The datatype: Literal Constructors: Overloaded - Contains an overloaded literal with a SrcSpan NonOverloaded - Contains non-overloaded literals with a SrcSpan NoLocation - currently unused. This data type is bound to change. This module is designed to capture ALL numeric literals for use in the plugin. When parsing Haskell source tree, certain cases result in getting a plain `Expr`. This means it is not annotated with a SrcSpan. As a result, we pass along a SrcSpan through most functions to keep track of the most recent SrcSpan encountered. This isn't a fool proof plan and may be removed in the future. * Implement Logic for actual CodeActions. The traversal of Haskell source has been turned into a rule. This rule generates hidden diagnostics which allow a user to run a code action over (most) literals. There is some "dead code" in the sense that some literals (prims to be exact) are not allowed to be changed, despite existing in code. The other "dead code" exists in the FormatType type. Certain extensions exist that alter how numbers are shown or act, we make note of those extensions but call them "NoFormat", which essentially means it's a NOOP. Most of the conversion is done via the Numeric module. There are plenty of "show" functions that provide the translation of numbers into the various formats. The only custom format is a slightly "dumb" NumDecimal-extension-function that generates three separate options, instead of using a numeric defined function. * Clarify source code comments. Reformat file structure. * Set up Test Scaffolding. * Miscellaneous clean-up. * Use Regex to Match Source Text. * Remove unneccessary diagnostics from Rule. * Update all stack versions to include new Plugin. * Clean up Conversion.hs and AlternateNumberFormat. Clean up function names and add supporting haddock documentation for exported functions. * Update cabal file bounds and add Github workflow test * Add plugin package to all cabal.project * Add plugin to test suite * Push SYB changes. Test.hs is merely for testing and will be removed prior to Merge. * Use Generics.SYB for parsing Literals from GHC Source. Updated Literals.hs to use custom SYB traversal of the AST. * Update hls-alternate-number-format-plugin.cabal * WIP: run tests * Test additions and GHC compatability changes. * Removed dead code. PrimLiterals were defined originally, however GHC source doesn't provide the source text. This is one of the needs of the plugin so we don't provide duplicate suggestions. Swapped `Maybe Text` to `Text`. Similar to above we now ignore all literals that have no source text attached. Swapped `Maybe SrcSpan` to `RealSrcSpan`. Again, similar to the previous points we now ignore literals that don't get a sourceSpan attached. Similarly, we drop any `UselessSpan`'s as those are also not very helpful. * Use Set to remove duplicates from CollectLiterals Result. Certain AST representations are traversed multiple times with the switch to SYB. Using Set allows to easily remove duplicate Literals from our result. Added a test suite to monitor for regressions. * Delete Test.hs Inadvertently commit this file. * Add README and bump version to 1.0.0.0. * Update HLS cabal file for version change. * Additional Tests. * Update Test file to use Path library function. * Miscellaneous Code Fixes. - Remove unneeded imports/functions. - Export only specified functions from each module (and add Haddock Comment if necessary) - Minor text changes for accuracy * Add README to Extra Source Files. * Update Extra Source Files to include Test Haskell Source files. * Update Features.md and Minor Bug Fix :) * Add new plugins to hackage workflow * Fix Bug caused by Minor Bug Fix. Co-authored-by: jneira <atreyu.bbb@gmail.com> Co-authored-by: Fendor <power.walross@gmail.com>
The following PR is for issue #1560. Currently all functionality is "complete", however there is a lot of dead code (code that exists but the plugin doesn't need to use). I am looking to get some feedback on the plugin itself, and hopefully through the feedback most of the dead code can be culled.
General Mechanics:
A new rule
CollectLiteralstraverses the GHC source tree to look for literals (Literals.hs). The rule returns the exhaustive list of literals and also a list ofFormatType.FormatType's are derived from currently active extensions as well as a base set that are valid no matter the extensions active.When a user invokes a
codeActionkeybind, thealternateNumberFormathandler will filter out only the literals that are in the current range of the invokedcodeAction. In the handler, the logic for generating possible alternatives is done, and each possibility is transformed into acodeActionsent to the user.Most of the alternate formats are provided by the
Numericpackage (show* functions).Potential Issues:
Literals.hstraverses nearly the entire source tree just to make sure all of the Literals are captured. Certain types of literals end up being discarded (Literals with no source text attached or source location). WhencodeActions are generated we just immediately throw those out. As a result theLiteralconstructors ofIntPrimLiteralandFracPrimLiteralaren't actually used.Conversion.hswe transform the ExtensionsNumericUnderscoresandNegativeLiterals(LexicalNegationin GHC > 9), intoNoFormatwhich essentially means we don't care about them right now. I'm not sure if generating cases for this would be useful or needed. (NegativeLiteralsmay not actually be useful to this plugin)NumDecimalsextension allows for literals such as1.23e5to be used places where Integer's are expected. I could've gone ahead and defaulted to using the Numeric package, but I tried to encode some logic to display different possible exponents. Ex.12.3e4,123.0e3, and1.23e5for123000.cabal.projectandhaskell-language-server.cabalbut please let me know if there is more to do.cabalfile as I was using Stack to build and I am not quite clear on how to generate these bounds effectively.Let me know what you think.