Code action: remove redundant constraints for type signature #692
Conversation
DenisFrezzato
force-pushed
the
code-action-remove-redundant-constraint
branch
from
b0e9906 to
c50e74b
Compare
| <&> (head >>> parseConstraints) | ||
|
|
||
| findConstraints :: T.Text -> T.Text -> T.Text | ||
| findConstraints contents typeSignatureName = contents |
There was a problem hiding this comment.
On a second thought, I've realised that this will fail if the type signature is not formatted as expected, for example with an arbitrary number of spaces or written over multiple lines (see also lines 608 and 611).
While I don't reckon it could brake the code, this is rather poorly handled. I see two ways to improve it:
- trivial one: make it a bit safer and do nothing if the code is not formatted as expected;
- trickier: improve the logic in order to handle any format. I'm not sure this would be achievable with regular expressions and
Textfunctions though, so maybe some text parsing should be involved?
What do you think?
There was a problem hiding this comment.
Usually do the trivial one now, and then handle the trickier one in a follow up PR if its a problem in practice.
There was a problem hiding this comment.
Doing the trivial one now is totally fine but we should do nothing if the formatting doesn’t match our expectations instead of crashing which the last and head calls will do.
There was a problem hiding this comment.
Nice work, thank you! As mentioned in the comment, I think it’s perfectly fine to ignore the case where the formatting doesn’t match what we expect but I don’t want to crash. Probably easiest to turn the partial functions into something returning a Maybe and then add that to the pattern guard.
| <&> (head >>> parseConstraints) | ||
|
|
||
| findConstraints :: T.Text -> T.Text -> T.Text | ||
| findConstraints contents typeSignatureName = contents |
There was a problem hiding this comment.
Doing the trivial one now is totally fine but we should do nothing if the formatting doesn’t match our expectations instead of crashing which the last and head calls will do.
|
@cocreature Yes, that is exactly what I was planning to do, but I didn't have time yet. Since I'm hanging here, is it fine with you if I add |
|
Yes |
Make the content parsing safe for type signature formatted with an arbitrary and unexpected number of spaces and/or line feeds.
…/ghcide#692) * Code action: remove redundant constraints for type signature * Handle peculiar formatting Make the content parsing safe for type signature formatted with an arbitrary and unexpected number of spaces and/or line feeds.
…/ghcide#692) * Code action: remove redundant constraints for type signature * Handle peculiar formatting Make the content parsing safe for type signature formatted with an arbitrary and unexpected number of spaces and/or line feeds.
…/ghcide#692) * Code action: remove redundant constraints for type signature * Handle peculiar formatting Make the content parsing safe for type signature formatted with an arbitrary and unexpected number of spaces and/or line feeds.
This closes #60.