#162 Console plugin extension. Allow Project Root Path shortcut in Error Pattern "filename".
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 ?
Ok just addressing my own comments:
gives shorter/cleaner code ?
Ticket moved from /p/jedit/patches/523/
Does this patch fix a bug? If so, please reference the ticket.
If your error pattern catches the full path or just part of it,
and you are using ProjectViewer as well as either FastOpen/SmartOpen,
ErrorList will first try to open the file at the incorrect location, and then after that, use a FileOpenerService to open it by filename (without the path).
The FileOpenerService will open your file using FastOpen or SmartOpen, so it shouldn't matter if the path is picked up incorrectly by the error matcher.
Does that feature not work for you??
http://plugins.jedit.org/plugindoc/ErrorList/#idp60160
Last edit: Alan Ezust 2014-04-16
Your particular patch makes Console depend on ProjectViewer for all uses of ErrorMatcher, changing it from an optional dependency to a regular dependency.
I don't think that is necessary now that we have the FileOpenerService.
No: this patch is an extension. That's why the title says "Console plugin extension", but that was probably a misnomer.
Thanks for pointing out FastOpen/SmartOpen, but at least for my project, this does not seem to work (checked with FastOpen).
I do not even know WHY it does not work. I tried to create a test case (project dir, C files, etc) but there it seems to work, maybe because the test case is too simple.
So I cannot tell you why it does not work for the particular project I tried it for. It just doesn't.
My intent was to make this just an option and not to force ProjectViewer as dependency. I just saw that the Console Plugin already uses ProjectViewer in "ConsolePlugin.java", so I did not really check out how to avoid that.
I think I fixed that in the newly created attachment.
Note: The Console Plugin already provides similar functionality for what I intended: It declares the environment variable $p (or %p% or ${p}) for the system shell it opens (if ProjectViewer is used and the active project is set).
I just wanted to get access to the ProjectViewer path in the "Error Pattern" suboptions of the Console Plugin, so that I am able to specify it in the "filename" text box in some way so that I can do something like
filename: #p#/$2
(for example), where #p# is then replaced by the active ProjectViewer path.
Please reconsider the freshly attached patch. I really believe it is highly useful...
Please try this version of ErrorList, which should use the FileOpenerService at the right times.
http://lazarus.oddiofile.com/jars/ErrorList.jar
If FileOpenerService does not work for you after testing my pre-release of ErrorList 2.2, please contact me or open a ticket against it.
Last edit: Alan Ezust 2014-04-21
Hello, It still does not work for me.
To make some progress, I now built everything from the SVN sources (jedit, ProjectViewer, ErrorList, Console,...).
I guess the ErrorList.jar you provided above is at least from "r23505", because the svn log there says "Fixing usage of FileOpenerService".
I could not download the ErrorList.jar from the link you provided, so I just directly compiled it out of the SVN sources (r23512).
FastOpen still does not work for me.
I finally created a test project to demonstrate.
Please see attached "prja.tgz".
The directory structure here is:
build/ <- subdir which contains Makefile and where objects are compiled
srca/ <- subdir with some source code
srcb/ <- subdir with some source code
script/deep/ <- subdir which houses scripts to build and clean project.
To build this mini project I open the Console, chdir to "script/deep/" and then run "./build". (to clean I use "./clean").
Building the mini project outputs three warnings:
../srca/file.c:5:3: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long long unsigned int’ [-Wformat]
../srcb/file.c:9:4: warning: assignment makes integer from pointer without a cast [enabled by default]
../srca/hello.c:5:3: warning: format ‘%d’ expects argument of type ‘int’, but argument 2 has type ‘long long unsigned int’ [-Wformat]
I am not able to get these into ErrorList with the right filenames with FastOpen and the Console Error Pattern.
I tried the following Error Pattern:
Error Regexp: (../)*([^:]*):([0-9]*):[^:]*: error: (.*)
Warning Regexp: (../)*([^:]*):([0-9]*):[^:]*: warning: (.*)
Filename: $2
Line number: $3
Error message: $4
For the first warning this will give
srca/file.c:6:assignment makes pointer ...
This does not seem to work together with FastOpen. Maybe, because I have got a leading "srca/". But this is necessary, because there is "srca/file.c" and "srcb/file.c".
As an alteranative I also tried the following Error Pattern:
Error Regexp: ([^:]*):([0-9]*):[^:]*: error: (.*)
Warning Regexp: ([^:]*):([0-9]*):[^:]*: warning: (.*)
Filename: $1
Line number: $2
Error message: $3
So this gives the full error filename as output by the compiler, but this still does not seem to help FastOpen.
With the patch I proposed I use:
Error Regexp: (../)*([^:]*):([0-9]*):[^:]*: error: (.*)
Warning Regexp: (../)*([^:]*):([0-9]*):[^:]*: warning: (.*)
Filename: #p#/$2
Line number: $3
Error message: $4
For the first warning, this will give
<project dir>/srca/file.c:6:assignment makes pointer...
which is exactly what I need.
So this just expands to the right path, without the need for FastOpen at all.
I re-attach the proposed patch, but this time directly against the SVN tree from the Console Plugin.
I tried to get the patch even more compact and still hope it might be included.
Last edit: lundril 2014-04-21
On Mon, Apr 21, 2014 at 8:31 AM, lundril lundril@users.sf.net wrote:
If you are testing svn trunk of console, you might also want to try out the
new feature I added, under Project Properties - Console
Now you can "run commands from here:"
On Mon, Apr 21, 2014 at 8:31 AM, lundril lundril@users.sf.net wrote:
That should work fine. I've tested it on the default error patterns.
FastOpen still does not work for me.
So it should not matter whether the full path appears as incorrect by
ErrorList, because AFTER YOU CLICK ON IT IN ERRORLIST, if it fails to find the file at that location, it should
pop up the FastOpen and enter SOMETHING into the fastopen field.
Do you see a FastOpen window pop up? Is anything entered into the filename
textfield?
Is your problem because you have because you have 2 files called "file.c"
in different folders and fastopen can't decide which one to open so it is
asking you to pick one?
I tried the following Error Pattern:
This is enough for me to reproduce your issue - I don't need a test
project, just the output and the regexes. And I haven't tested your regex
because generic parses it fine for me.
The way I test it is by just "cat erroroutputfile.txt" in console and
ErrorList parses the file just as if it was the output of an actual command.
Please describe what happens when you say "it does not seem to work", so I
can better understand what is happening on your end.
Are you saying another file is opened instead? Or no fastopen window shows
up, or what?
I don't understand what this means either. No fastopen window pops up after you click on an error? or the wrong file opens?
Last edit: Alan Ezust 2014-04-21
Not sure if replying via E-Mail works, so I just re-post it here:
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.
On Mon, Apr 21, 2014 at 04:17:42PM +0000, Alan Ezust wrote:
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.
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
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 ?
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 ;-).
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.
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.