GRECLIPSE
  1. GRECLIPSE
  2. GRECLIPSE-1316

Unexpected indentation when pressing Ctrl + Alt + Down on lines in the editor

    Details

    • Type: Bug Bug
    • Status: Open Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 2.6.0.Release
    • Fix Version/s: None
    • Component/s: Editor, Formatting
    • Labels:
    • Number of attachments :
      0

      Description

      One issue that I have come across while editing is the following:

      [key:1]
      

      Pressing Ctrl + Alt + Down (Copy Line(s) Down) 5 times consecutivesly while positioned on the line will produce the following in the editor:

      [key:1]
       [key:1]
        [key:1]
         [key:1]
          [key:1]
           [key:1]
      

      This behavior is also observed with the following snippet:

      call name: 'foo'
      

      Which produces:

      call name: 'foo'
          call name: 'foo'
              call name: 'foo'
                  call name: 'foo'
                      call name: 'foo'
                          call name: 'foo'
      

      Note, however, that this does not happen with:

      call name: 111
      

      I'm not sure of all the conditions this happens under, but it happens frequently enough to be really annoying.

        Activity

        Hide
        Bob Tiernay added a comment -

        Another example is:

        [               'key':1]
        

        Which produces:

        [               'key':1]
                        [               'key':1]
                                        [               'key':1]
                                                        [               'key':1]
                                                                        [               'key':1]
                                                                                        [               'key':1]
        
        Show
        Bob Tiernay added a comment - Another example is: [ 'key':1] Which produces: [ 'key':1] [ 'key':1] [ 'key':1] [ 'key':1] [ 'key':1] [ 'key':1]
        Kris De Volder made changes -
        Field Original Value New Value
        Assignee Kris De Volder [ kdvolder ]
        Hide
        Andrew Eisenberg added a comment -

        This particular issue comes from the implementation in org.eclipse.jdt.internal.ui.javaeditor.JavaMoveLinesAction and the implementation in org.eclipse.jdt.internal.ui.javaeditor.IndentUtil. My guess is that if we can sub-class JavaMoveLinesAction and use our infrastructure instead and figure out where to plug in the new pieces, then this will be solved.

        Show
        Andrew Eisenberg added a comment - This particular issue comes from the implementation in org.eclipse.jdt.internal.ui.javaeditor.JavaMoveLinesAction and the implementation in org.eclipse.jdt.internal.ui.javaeditor.IndentUtil . My guess is that if we can sub-class JavaMoveLinesAction and use our infrastructure instead and figure out where to plug in the new pieces, then this will be solved.
        Hide
        Andrew Eisenberg added a comment -

        I think the functionality in org.codehaus.groovy.eclipse.refactoring.formatter.GroovyIndentationService and org.codehaus.groovy.eclipse.editor.GroovyAutoIndentStrategy could be used to implement this.

        Show
        Andrew Eisenberg added a comment - I think the functionality in org.codehaus.groovy.eclipse.refactoring.formatter.GroovyIndentationService and org.codehaus.groovy.eclipse.editor.GroovyAutoIndentStrategy could be used to implement this.
        Hide
        Andrew Eisenberg added a comment -

        Here is how to hook in our functionality. In org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor.createActions(), around line 1105, the move lines commands are created. The GroovyEditor class subclasses the createActions command and provides some infrastructure for replacing JDT commands with our own. Just need to follow the existing code with more of the same.

        Show
        Andrew Eisenberg added a comment - Here is how to hook in our functionality. In org.eclipse.jdt.internal.ui.javaeditor.CompilationUnitEditor.createActions() , around line 1105, the move lines commands are created. The GroovyEditor class subclasses the createActions command and provides some infrastructure for replacing JDT commands with our own. Just need to follow the existing code with more of the same.
        Andrew Eisenberg made changes -
        Labels help-requested
        Hide
        Andrew Eisenberg added a comment -

        Problem is specifically with org.eclipse.jdt.internal.ui.javaeditor.JavaMoveLinesAction.runWithEvent(Event). The call to IndentUtil.indentLines(..) indents the result, but in this particular case, the result should not be indented.

        There is no way to swap in our own IndentUtil class, so we will have to reimplement this behavior to fix the problem. The solution may be as simple as copying and just not performing the call to indent (there would be other cases that this doesn't handle, but I think it would be good enough for what we need to do).

        Show
        Andrew Eisenberg added a comment - Problem is specifically with org.eclipse.jdt.internal.ui.javaeditor.JavaMoveLinesAction.runWithEvent(Event) . The call to IndentUtil.indentLines(..) indents the result, but in this particular case, the result should not be indented. There is no way to swap in our own IndentUtil class, so we will have to reimplement this behavior to fix the problem. The solution may be as simple as copying and just not performing the call to indent (there would be other cases that this doesn't handle, but I think it would be good enough for what we need to do).
        Hide
        Bob Tiernay added a comment -

        Hey, I thought you were well versed in AspectJ. Can't you devise some sort of load time weaving to instrument a fix?

        In all seriousness, is an option for situations like this where you can't effect the runtime semantics of JDT by conventional means?

        Show
        Bob Tiernay added a comment - Hey, I thought you were well versed in AspectJ. Can't you devise some sort of load time weaving to instrument a fix? In all seriousness, is an option for situations like this where you can't effect the runtime semantics of JDT by conventional means?
        Hide
        Andrew Eisenberg added a comment - - edited

        Our patch only covers JDT core and it is complex to maintain. We would need to patch a new plugin to fix this. And the benefit that we would get for this is not worth the effort we would need to maintain the patch from here until eternity.

        I would be fine with copying the code into groovy-eclipse.

        Show
        Andrew Eisenberg added a comment - - edited Our patch only covers JDT core and it is complex to maintain. We would need to patch a new plugin to fix this. And the benefit that we would get for this is not worth the effort we would need to maintain the patch from here until eternity. I would be fine with copying the code into groovy-eclipse.

          People

          • Assignee:
            Kris De Volder
            Reporter:
            Bob Tiernay
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated: