(17/26) 162 - Console plugin extension. Allow Project Root Path shortcut in Error Pattern "filename".

Ok just addressing my own comments:

- Alternative version, which only modifies ErrorMatcher.match() . This
gives shorter/cleaner code ?
- Use #p# as tag, to be less like an XML tag.

project_dir_in_error_pattern_2.patch (1.1Kio)

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

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.

> Does this patch fix a bug? If so, please reference the ticket.

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.

> Your particular patch makes Console depend on ProjectViewer
> for all uses of ErrorMatcher, changing it from an
> optional dependency to a regular dependency.

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...

project_dir_in_error_pattern_3.patch (1.2Kio)

1. the unused import in ConsolePlugin.java is a mistake and I just removed it from trunk. thanks for letting me know.
2. i just did some testing of FileOpenerService and it's not working for me either. After some debugging I fixed it in ErrorList.
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.

- **status**: open --> closed-rejected

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.

prja.tgz (669B)

svn_project_dir_in_error_pattern.patch (971B)

On Mon, Apr 21, 2014 at 8:31 AM, lundril <lundril@users.sf.net> wrote:

> Hello, It still does not work for me.
>
> To make some progress, I now built everything from the SVN sources (jedit,
> ProjectViewer, ErrorList, Console,...).
>


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:

> 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).
>

That should work fine. I've tested it on the default error patterns.


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.
>


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:
>
> 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 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.



> 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".
>

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?



> 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.
>

I don't understand what this means either. No fastopen window pops up after you click on an error? or the wrong file opens?

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.

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.

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.

- **status**: closed-rejected --> open

> 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 ?

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 ;-).

project_dir_in_error_pattern_4.patch (920B)

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.