Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Line debugger fails on homonyms #153

Open
vincentb1 opened this issue Aug 14, 2018 · 3 comments
Open

Line debugger fails on homonyms #153

vincentb1 opened this issue Aug 14, 2018 · 3 comments

Comments

@vincentb1
Copy link

vincentb1 commented Aug 14, 2018

This is a duplicate of bug №30 on Sourceforge, and I am coming with a fix.

When two classes have exactly the same name, but are from different packages, then the line debugger may fail to find the correct file.

Example: in jPicEdt there are several formatters (for instance the same drawing can be formatted to PSTricks, or to PGF/TikZ, as per user's choice), so to format e.g. an Ellipse to TikZ you have a class jpicedt.format.output.tikz.PicEllipseFormatter, and to format it to PSTricks you have a class, with the same interface, jpicedt.format.output.pstricks.PicEllipseFormatter.

You may pr may not argue that this is a bad practrice to do so, but anyway this is a perfectly valid construct from the Java point of view, so JDEE should be compatible with this.

@vincentb1
Copy link
Author

Attached is a patch file.

SF_bug30_GH_bug153.diff.zip

This is a starting point, it suffers several remaining issues:

  • first it has several spurious changes due to some latin1/utf8 transcoding stuff, I need to make some hook so that when I open an Elisp file in the JDEE work area coding scheme remains UTF-8
  • also it has an issue that it tackles two things at a time:
    • it solves the bug
    • but it also brings a code optimisation by using a cache jdee-source-path-hashmap-cache hashmap.
  • finally when fixing the bug I realize the the root cause why I fell into the bug is the poor documentation of jdee-sourcepath variable, and this patch does not fix this issue.

This code optimisation is incomplete, because I think that this would bring some problems when you have several instances of the same projects in parallel, ie the same full classname in one project would map to a source file in another project. I did not find any simple way to solve this. Probably one way would be to combine the full-classname with a unique project id in the map keys. Another more radical way would be to rewrite the whole function as a method of the debugger object and make the hashmap a member of this object,

OK, it would probably be more reasonable to have a phased approach and fix first the bug w/o the optimisation.

@vincentb1
Copy link
Author

Concerning the documentation of jdee-sourcepath, here is what I mean. To go on with the jPicEdt example which I gave in the orginal bug submission, I have jdee-sourcepath containing this two paths: /jpicedt-project/jpicedy/format/output/tikz and /jpicedt-project/jpicedt/format/output/pstricks, the current JDEE code bogously selects the first path in the list w/o checking that the classname is jpicedt.format.output.pstricks.PicEllipseFormatter, so it wrongly sends the line debugger at the source of jpicedt.format.output.tikz.PicEllipseFormatter .
This would be solved if the jdee-sourcepath just contained /jpicedt-project (that would incidentally also make it quite simpler and maintainable, because currently it is a very long list of paths for jPicEdt…). So in a nutshell my problem could be seen as a wrong configuration of JDEE because it could be worked around by configuring JDEE otherwise without any code fix.
However, I believe that is still a problem to be fixed in JDEE, especially the docstring of jdee-sourcepath should be improved — I will provide another patch to propose a fix for that.

@pwojnowski
Copy link
Member

Hi. Sorry for late reply, but it took me some time to write tests for this part and wrap my head around the issue.

IIUC what you described is valid case. Even when there is more than one source file matching class name then still the correct one should be selected - based on Fully Qualified Name. What's more, the location of a source file on the source path doesn't have to match directory structure, therefor your configuration, although uncommon and discouraged, is still valid.

Of course, the simplest solution is to correct the jdee-sourcepath as you've described. Then the correct file will be found.
The other one is to correct searching for the correct source file. To make it really cover all possible cases it would need to look into java files and check their package declaration. But I think it's not worth doing this as such configurations are really rare cases and the searching (without external indexes) would be really slow.

Regarding your patch. Please create Pull Request were we could review and discuss it. I created a few tests for jdee-find-class-source-file so it should be easier to verify if it actually works.
IMHO it would be good to not add caching in this PR, especially that it's coupled with the rest of the code. So, please create Pull Request.

Thanks! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants