#3 editor action refactoring

closed-fixed
nobody
None
5
2006-08-04
2006-06-24
Jae Gangemi
No

the attached patch contains a refactoring of the action
classes to split the IEditorDelegate implementation
from the implentations of the actions themselves.

in addtion, it also upgrades Devel::Refactor to version
0.05, fixes a bug with the refactoring action, and also
addresses the next/prev nav items not working (see
http://sourceforge.net/tracker/index.php?func=detail&aid=1508722&group_id=75859&atid=545277\)

Discussion

  • Jae Gangemi

    Jae Gangemi - 2006-06-24

    patch, image

     
  • Jan Ploski

    Jan Ploski - 2006-06-25

    Logged In: YES
    user_id=86907

    Argh! You reformatted PerlEditor.java. Please undo the
    reformatting and attach the file with whatever actual
    changes were necessary. The problem is that the
    reformatting breaks history - one is no longer able to
    compare two revisions before and after the reformatting.

    I suggest the following policy for reformattings: if you
    refactor a source file beyond recognition anyway, then
    reformatting it does not matter (the history is lost
    anyway). OTOH, if you make just small changes, make them
    appear small for diff => no reformatting. Ok?

     
  • Jae Gangemi

    Jae Gangemi - 2006-06-25

    diff

     
  • Jae Gangemi

    Jae Gangemi - 2006-06-25

    Logged In: YES
    user_id=11978

    d'oh - i thought i only formatted the classes that were
    refactored and/or new. new patch is attached.

     
  • Jan Ploski

    Jan Ploski - 2006-06-25

    Logged In: YES
    user_id=86907

    I committed your patch. Here are some more comments:

    Please keep your code 3.1-compatible. The class FileLocator
    is not available in Eclipse 3.1, so I got rid of the
    reference.

    Is there a reason why in MANIFEST.MF Eclipse-LazyStart has
    changed to Eclipse-AutoStart? Anyway, it caused the plug-in
    to fail under 3.1, so I reverted your change.

    I don't understand the comment in CritiqueSourceAction. For
    me it sounds like "something doesn't work, and I don't know
    why, so I tried something else". But I suppose it is just
    work in progress.

    Please don't use CVS keyword expansion (e.g., @version tag
    with $Revision$, $Date$).

    OpenDeclarationAction - I reverted the reformatting. Also
    reverted it in some other cases.

    I also changed occurrenceIndicationColor back to match JDT
    (I don't see why we should be different than JDT here... or
    has it changed from 3.1 to 3.2?).

    The next/prev occurrence navigation still does not work
    (buttons stay disabled). Because you wrote that you fixed
    it, I suspect this too might have to do with the 3.1/3.2
    difference.

     
  • Jae Gangemi

    Jae Gangemi - 2006-06-25

    Logged In: YES
    user_id=11978

    The FileLocator and MANIFEST change were marked as
    deprecated, but i didn't realize it was for 3.2 only, my bad.

    the next/prev buttons work in 3.2 (the patch i submitted
    before was incorrect anyway) - the entire annotationSupport
    definition came from the jdt "example", so it appears they
    have changed the color).

    as for the comment in CritiqueSourceAction - perlcritic does
    not seem to like taking the source input via STDIN when
    invoked from the PerlExecutor, but i can't seem to track
    down what causes this b/c if i hack the Perl::Critic module
    to display the passed source code, it's identical. to get
    around this, i just run perlcritic against the actual file
    on disk.

     
  • Jan Ploski

    Jan Ploski - 2006-06-25

    Logged In: YES
    user_id=86907

    Regarding the stdin problem: see explanations of the
    "broken pipe" condition in ProcessExecutor.java. I updated
    your code to use the same work-around as PerlValidator.

     
  • Jae Gangemi

    Jae Gangemi - 2006-06-25

    Logged In: YES
    user_id=11978

    that doesn't fix the issue, here is the output from
    perlcritic trying to feed the editor text via stdin:

    Perl Process stderr: Warning: Can't parse code: Unknown
    error returned by PPI::Lexer for 'stdin' at
    /usr/bin/perlcritic line 203

    i tried hacking the Perl::Critic code to print out STDIN,
    but i can't see any differences in the code.

    also, running the "validate syntax" action via the shortcut
    and/or menu option doesn't cause the decorator to update the
    entry in the navigator view any longer. i can't seem to find
    anywhere that the refactoring could have changed this, so if
    you have any ideas, i'd like to hear them b/c i can't get
    this working w/ the critique stuff.

    i thought it was enough to just create the marker and set
    the attributes, but perhaps there is something else that i'm
    missing.

     
  • Jan Ploski

    Jan Ploski - 2006-06-25

    Logged In: YES
    user_id=86907

    Which source file are you testing the perlcritic against -
    can you attach it here? I got an error similiar to what you
    describe for one file (syntax.pl from org.epic.perleditor-
    test), but it works ok for another (Twig.pm). However, in
    my case the error is unrelated to whether I use a command-
    line call or an Eclipse action - it just seems that
    perlcritic fails on syntax.pl (which I can imagine, as it
    is not well-formed Perl code).

     
  • Jan Ploski

    Jan Ploski - 2006-06-25

    Logged In: YES
    user_id=86907

    Resource decorations are only updated when the file is
    saved - AFAIK, it has always been so. The same is true for
    Java files: problems reported as you type do not update the
    decoration in Package Explorer until you save the file.

     
  • Jae Gangemi

    Jae Gangemi - 2006-08-04
    • status: open --> closed-fixed
     

Log in to post a comment.