Skip to content
This repository was archived by the owner on Oct 7, 2020. It is now read-only.

Support for Pattern Synonyms #1152

Merged
merged 1 commit into from
Apr 4, 2019
Merged

Support for Pattern Synonyms #1152

merged 1 commit into from
Apr 4, 2019

Conversation

anton-dessiatov
Copy link
Contributor

Please don't merge as is, let's wait if haskell/lsp#143 gets merged.

The thing is that when used on modules that have pattern synonym declarations, hie kept crashing due to error call at line 693 of src/Haskell/Ide/Engine/Plugin/GhcMod.hs file. I have fixed this, but it required to introduce a new SymbolKind in haskell-lsp package.

This pull request reflects current state of things including referencing my forked version of haskell-lsp. Once required changes to haskell-lsp get released, I'm going to update this pull request and remove my fork reference.

Meanwhile if there are problems with an approach I've taken, I have plenty of time to fix them.

@anton-dessiatov
Copy link
Contributor Author

It turns out that one does not simply add a new Symbol Kind to Microsoft LSP. I have removed custom Symbol Kind and ended up using Function kind for pattern synonyms. Please merge this pull request if you consider it appropriate.

@anton-dessiatov anton-dessiatov changed the title WIP: Support for Pattern Synonyms Support for Pattern Synonyms Apr 2, 2019
@anton-dessiatov
Copy link
Contributor Author

Whoops, it fails for GHC 8.6+. Will fix now.

@anton-dessiatov
Copy link
Contributor Author

anton-dessiatov commented Apr 2, 2019

Fixed, but leaved out an 'error' call in case GHC 8.6+ pattern synonym binding evaluates to XPatSynBind. I'm no GHC expert and could use some hint on under which circumstances could it ever evaluate to XPatSynBInd. In my testing environment I never managed to witness that case (tested with GHC 8.6.4).

I would say that anyway it's better than before and could probably be merged.

Copy link
Collaborator

@lukel97 lukel97 left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good. Could you add a small test case for this in test/functional/SymbolsSpec.hs?

@anton-dessiatov
Copy link
Contributor Author

Sure! Added tests.

@lukel97
Copy link
Collaborator

lukel97 commented Apr 4, 2019

@alanz is this good to merge?

@alanz alanz merged commit 1a58d5c into haskell:master Apr 4, 2019
@alanz alanz added this to the 2019-04 milestone May 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants