Skip to content

Conversation

@marcuss
Copy link
Contributor

@marcuss marcuss commented Mar 6, 2021

One big thing to review here is the possibility to remove the fixed guess that all jars/projects have source code in a folder like: java/main/src

@kgilpin
Copy link
Contributor

kgilpin commented Mar 6, 2021 via email

@marcuss
Copy link
Contributor Author

marcuss commented Mar 6, 2021

Please don’t assume anything about the code structure of the project. That will make things harder in the client (VSCode) not easier. Just report the file path as it is reported to you by Java.

On Sat, Mar 6, 2021 at 3:53 PM Marcus S. @.***> wrote: One big thing to review here is the possibility to remove the fixed guess that all jars/projects have source code in a folder like: java/main/src ------------------------------ You can view, comment on, or merge this pull request online at: #57 Commit Summary - Merge pull request #1 from applandinc/master - Fix Issue 56, added better way to extract the file path. - Fix Issue 56, added better way to extract the file path. File Changes - M src/main/java/com/appland/appmap/output/v1/CodeObject.java https://github.com/applandinc/appmap-java/pull/57/files#diff-6478ffcbeb325a48349797051cf4c68dd71ae84fa8554336fab6534f8a6b4da7 (51) - A src/test/java/com/appland/appmap/output/v1/CodeObjectTest.java https://github.com/applandinc/appmap-java/pull/57/files#diff-de82ebfbb765ae0ff41331ca97741dbb4e817d8b2771d85553698528bf1bab98 (31) - A src/test/java/com/appland/appmap/output/v1/testclasses/Anonymous.java https://github.com/applandinc/appmap-java/pull/57/files#diff-a51e0656c0dffa3495e50d3e76b4077264accebab31f47debefac8c1370241c8 (13) - A src/test/java/com/appland/appmap/output/v1/testclasses/ExampleInnerClass.java https://github.com/applandinc/appmap-java/pull/57/files#diff-e39e6833667c7ae6ad2c1b9a7bde9bac444fb4e7dae9138d8de663a53752cc1c (11) - M src/test/java/com/appland/appmap/test/util/MethodBuilderTest.java https://github.com/applandinc/appmap-java/pull/57/files#diff-274eb8963d1765f684ef34a489d61934994443b503f7636d3b6c7ea133da073a (4) Patch Links: - https://github.com/applandinc/appmap-java/pull/57.patch - https://github.com/applandinc/appmap-java/pull/57.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#57>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAVC6ZIREGO6DNAIXST55DTCKI5VANCNFSM4YXDCYNA .

Exactly!!

I actually just remove that assumption from the code

thanks @kgilpin

@marcuss marcuss changed the title Issue 56 get source file path improvement. [READY TO REVIEW] Issue 56 get source file path improvement. Mar 9, 2021
@marcuss
Copy link
Contributor Author

marcuss commented Mar 16, 2021

@dustinbyrne @kgilpin Could you please take a look at this, I believe is ready to merge

@dustinbyrne dustinbyrne force-pushed the master branch 4 times, most recently from 02054f7 to 3f2c0e7 Compare March 16, 2021 21:50
Comment on lines +12 to +16
@Test
public void getSourceFilePath_for_RegularClass() throws NotFoundException {
CtClass testCtClass = ClassPool.getDefault().get("com.appland.appmap.ExampleClass");
assertEquals("com/appland/appmap/ExampleClass.java", CodeObject.getSourceFilePath(testCtClass));
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unlikely, but is there any way of us capturing the source path relative to the project root? If we wanted to open this file, we'd need to know if it was located in src/java/test or src/java/main. We could choose a file with the best partial match but without the full relative path we'll always have edge cases.

🤔

@dustinbyrne
Copy link
Collaborator

Moving to #62

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

Successfully merging this pull request may close these issues.

3 participants