rikvdkleij / intellij-haskell

IntelliJ plugin for Haskell
https://rikvdkleij.github.io/intellij-haskell/
Apache License 2.0
1.31k stars 94 forks source link

Extend selection is broken #398

Open sir4ur0n opened 5 years ago

sir4ur0n commented 5 years ago

Hey,

Extend selection feature seems broken, which makes me think the parser is bugged? I'd assume this feature works OOTB in Intellij based on parsing.

Example:

module Foo where

foo :: IO ()
foo = putStrLn ("Foo")

bar :: Int
bar = 42

Put your cursor somewhere on the string "Foo" and use Extend selection 3 times. It should select ("Foo") but instead it selects "Foo") i.e. not the left parenthesis. Do it another time: it should select putStrLn ("Foo") but instead it selects the whole line foo = putStrLn ("Foo").

Now put your cursor on putStrLn and extend selection 3 times. This should select putStrLn ("Foo") but instead selects putStrLn (. Extend selection again: this selects putStrLn ("Foo" which doesn't make any sense.

By the way the same kind of issues happens on types (e.g. the line foo :: IO ()).

Last issue: put your cursor on 42 and extend selection 4 times. I think it would make sense to select all functions but not imports/module declaration, just like in Java once you extended to the class, it doesn't select yet imports/package declaration.

Besides Extend selection feature being broken, I'm wondering if it's not just the tip of the iceberg of broken parsing. This may lead to other bugs/issues?

rikvdkleij commented 5 years ago

Thanks for reporting!

I'd assume this feature works OOTB in Intellij based on parsing.

No, that is not the case. OTOB it does not behave as you expect. I mainly use this feature to select an expression and surround it with parens. OTOB it selects the whole expression (including the left hand side) almost immediately,

I do not agree that extend selection is completely broken. The examples you give are related to parens (except for last one).

AFAICS the parser itself behaves okay.

rikvdkleij commented 5 years ago

I will fix the issue with parens.

Last issue: put your cursor on 42 and extend selection 4 times. I think it would make sense to select all functions but not imports/module declaration, just like in Java once you extended to the class, it doesn't select yet imports/package declaration.

Will try to fix that also.

sir4ur0n commented 5 years ago

My bad, in my head it made sense Intellij provided this feature for free based on parsing result 😞

Thanks for your reply 😄

rikvdkleij commented 5 years ago

just like in Java once you extended to the class, it doesn't select yet imports/package declaration.

I do not see that behavior in Java code. It select also the imports and package declaration.

sir4ur0n commented 5 years ago

It does, but separately. Consider the following class:

package foobar;

import java.math.BigDecimal;

public class Foo {

  public BigDecimal bar() {
    return BigDecimal.ZERO;
  }
}

Put your cursor on ZERO and extend selection 9 times: the class is selected but not the imports/package declaration. Extend selection 3 more times and then both imports and package declaration are taken.

rikvdkleij commented 5 years ago

Aah, I see, I misunderstood you.

Current fix is without that, it goes from top declaration to module declaration. Is that really an issue? I do not see that as a problem.

rikvdkleij commented 5 years ago

Should be solved in beta46.

sir4ur0n commented 5 years ago

The behavior seems a lot better, however it's still weird/bugged for parenthesis.

Using the same example file as in my original issue:

I think after 3 selection extensions, it should select ("Foo"), i.e. be symmetrical on parenthesis.

rikvdkleij commented 4 years ago

To solve this issue properly the parser has to be changed.