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

feat: allow to exclude ABI by source path #1160

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Vampire
Copy link
Contributor

@Vampire Vampire commented Apr 8, 2024

Following up the comment in #1141:

I don't recall. I think this was implemented by someone else and they didn't have the bandwidth to implement that component, so left it commented out as a reminder. If you want to contribute it, I'm happy to take a look.

I had a quick look at the history.
#158 introduce these changes with the comment that for it to work the source file location is missing up to now and the two todos at e4e1d2a#diff-44d4684fe90105b8c2f6bfcb9ed566be03e60c3f2464db27b22fd8b0c6f5c207R159-R162 and e4e1d2a#diff-2bacf6be9cf95dbf682271812bc7338662b3ccaf7952b6880d65d0f89c6fe2e0R74-R75 remained.

The second of these todo which is about missing source location you seem to have fixed in 7d006ca#diff-2bacf6be9cf95dbf682271812bc7338662b3ccaf7952b6880d65d0f89c6fe2e0L117-L118, but just incompletely, as it only puts the source file name in there as read from the class file, but not the source path as initially intended and necessary for this feature.

This is a first shot at an implementation that realizes the feature. Please have a look. Of course tests are missing, but I'd like to get feedback first, especially as the logic only provides the correct sourcefiles with an heuristic on a best-effort basis. If you totally dislike it and don't have a better idea, it might not be worth putting more effort into it.

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.

None yet

1 participant