#162 Console plugin extension. Allow Project Root Path shortcut in Error Pattern "filename".

4.3
open
Alan Ezust
Console (1)
5
2014-04-23
2014-04-16
lundril
No

This patch is against Console-5.1.3.zip.

What it is about:

I build a C/C++ project with traditional "make".
The C/C++ compiler is called with relative paths.
So I get (for exmple) the following warning messages in the console output:

../../mod1/src/access/checkit.c:1381:23: warning: variable ‘excess’ set but not used

I use the following warning pattern to catch this:

../../([^\:]+)\:([^\:]+)\:. warning\:(.)

Now I have got several copies of the C/C++ project on my PC, but of course I only have got one Error Pattern to catch the warnings for the builds in all the different project directories.

So I want to be able to somehow replace the "../../" with my Project Root path in the Error Pattern matcher.

The following patch allows this by including <p> in your Error Pattern "filename".

For example like this:
filename: <p>/prja/$1

The <p> will be replaced by the Project Root path from the Project Viewer, when the project is build and the error and warning messages are collected in the Error List.

So for example the filename will be modified to:

/home/lundril/checkouts/copy_a/prja/mod1/src/access/checkit.c

because my Project Root is "/home/lundril/checkouts/copy_a".

This patch is intended as a basis for discussion.
TBD/Notes:

  • Maybe using <p> as magic tag to be replaced is not a good idea ?
    (because it is too close to XML tags ?).
    Note: Using $p does not seem to work, because this seems to conflict with
    the regex parser.
  • Maybe more/different tags might also make sense ?
  • Currently ErrorMatcher.matchLine() is modified to use the "expanded"
    filename (fileBackrefExp). Maybe it is better to instead further modify
    ErrorMatcher.match() around the call to MiscUtilities.constructPath() ?
    Maybe leads to cleaner code and completely avoids the
    fileBackrefExp declaration ?
1 Attachments

Discussion

<< < 1 2 (Page 2 of 2)
  • lundril
    lundril
    2014-04-21

    Not sure if replying via E-Mail works, so I just re-post it here:

    Let me clarify what I meant. You have to CLICK on the error (even if it is
    displaying the wrong filename) to see anything happen with
    FileOpenerService.

    Ahh. I should have found that out myself.
    I just looked at the filenames but did not even try to click on them.

    So clicking on them works (FastOpen pops up).
    In case the filename is unambiguous (like the "hello.c" it just opens the right file, so that's fine.

    But in the case of "srca/file.c", "srcb/file.c" this still is a problem:
    Because FastOpen cannot decide which of these two files to open (since they
    have the same basename), that basically means you have to select the right
    file for each warning in these files. So to be more precise: If I have got
    two warnings in "srca/file.c", then I have to use FastOpen for each of
    these two warnings.

    The patch I proposed just completely circumvents the problem by directly
    providing the correct absolute path.
    So on the one hand this means the ErrorList window shows the right path
    and additionally it just works as expected.

    So I would say FastOpen almost solves the problem.
    I still think, the patch I proposed just does it in a lot more direct manner.

     
  • lundril
    lundril
    2014-04-21

    On Mon, Apr 21, 2014 at 04:17:42PM +0000, Alan Ezust wrote:

    Let me clarify what I meant. You have to CLICK on the error (even if it is
    displaying the wrong filename) to see anything happen with
    FileOpenerService.

    Ahh. I should have found that out myself.
    I just looked at the filenames but did not even try to click on them.

    So clicking on them works (FastOpen pops up).
    In case the filename is unambiguous it just opens the right file.

    But in the case of "srca/file.c", "srcb/file.c" this still is a problem:
    Because FastOpen cannot decide which of these two files to open (since they
    have the same basename), that basically means you have to select the right
    file for each warning in these files. So to be more precise: If I have got
    two warnings in "srca/file.c", then I have to use FastOpen for each of
    these two warnings.

    The patch I proposed just completely circumvents the problem by directly
    providing the correct absolute path.
    So on the one hand this means the ErrorList window shows the right path
    and additionally it just works as expected.

     
  • Alan Ezust
    Alan Ezust
    2014-04-21

    Exactly, FileOpenerService only works for unambiguous filenames, while your patch does in fact set the correct path based on the project variable.
    Glad to see that FileOpenerService almost solves your problem!

    I am thinking that we can avoid adding the direct dependency on projectviewer if we can somehow use something like a bsh/getProjectRoot.bsh script instead of putting the PV classes directly in ErrorMatcher.java. If you can do that, then there is no trade-off to accepting your patch.

     
    Last edit: Alan Ezust 2014-04-21
  • Alan Ezust
    Alan Ezust
    2014-04-21

    • status: closed-rejected --> open
     
  • lundril
    lundril
    2014-04-22

    I am thinking that we can avoid adding the direct dependency
    on projectviewer if we can somehow use something like a
    bsh/getProjectRoot.bsh script instead of putting the PV classes
    directly in ErrorMatcher.java.

    I think I am still missing something here:
    The code in ConsolePlugin.java, method runProjectCommand() right now reads:

            // {{{ runProjectCommand() method
            private static void runProjectCommand(View view, String prop) {
                    if (jEdit.getPlugin("projectviewer.ProjectPlugin") == null) {
                            GUIUtilities.error(view, "console.pv.not-installed", null);
                            return;
                    }
    
                    projectviewer.vpt.VPTProject project =
                            projectviewer.ProjectViewer.getActiveProject(view);
                    if (project == null) {
                            GUIUtilities.error(view, "console.pv.no-active-project", null);
                            return;
                    }
    

    which is the same kind of code as I used in the last

    https://sourceforge.net/p/jedit/plugin-patches/_discuss/thread/4cbbec1c/91d7/attachment/svn_project_dir_in_error_pattern.patch

    patch.
    So I guess something about the runProjectCommand() method has to be different, because otherwise why does the proposed patch create more dependency on ProjectViewer, than what the runProjectCommand() method already does ?

     
  • lundril
    lundril
    2014-04-22

    Ok here is another try, this time using BeanShell to get the project path.
    That code is the same as what SystemShell.getVariableValue() uses.

    This patch is now even shorter than before ;-).

     
  • Alan Ezust
    Alan Ezust
    2014-04-22

    Console is supposed to be able to run without ProjectViewer (although it's been a while since I've tested it that way). And most of the PV code that is in Console was designed with an optional dependency in mind. An import at the top can cause problems, and other things can too. At the moment, Console doesn't work for me without ProjectViewer for a reason I have not yet determined.

     
  • Alan Ezust
    Alan Ezust
    2014-04-22

    Nice that it is getting smaller/simpler but I don't want to pay a runtime penalty if I don't use this feature. So it should check first if the string contains #p# before it does an extra beanshell eval.

     
<< < 1 2 (Page 2 of 2)