feat: allow to exclude ABI by source path#1160
Open
Vampire wants to merge 1 commit intoautonomousapps:mainfrom
Open
feat: allow to exclude ABI by source path#1160Vampire wants to merge 1 commit intoautonomousapps:mainfrom
Vampire wants to merge 1 commit intoautonomousapps:mainfrom
Conversation
8d055f3 to
12c808b
Compare
12c808b to
829b357
Compare
829b357 to
f7e5f4d
Compare
f7e5f4d to
5776b71
Compare
5776b71 to
579967f
Compare
579967f to
7a282a3
Compare
Owner
autonomousapps
left a comment
There was a problem hiding this comment.
Sorry for the long delay; it's been a busy year. I'm open to this change. With some tests, I'd be happy to see it merged.
Comment on lines
+145
to
+159
| val sourceFileName = clazz.sourceFile ?: "${clazz.name.substringAfterLast('/')}." | ||
| val clazzPackageReversed = clazz.name.substringBeforeLast('/').split('/').reversed() | ||
| val sourceFile = sourceFilePackageReversedBySourceFile | ||
| .filterKeys { it.name.startsWith(sourceFileName) } | ||
| .maxByOrNull { (_, sourceFilePackageReversed) -> | ||
| sourceFilePackageReversed | ||
| .asSequence() | ||
| .zip(clazzPackageReversed.asSequence()) | ||
| .takeWhile { (sourceFilePart, clazzPart) -> sourceFilePart == clazzPart } | ||
| .count() | ||
| } | ||
| ?.key | ||
| ?.invariantSeparatorsPath | ||
| ?: sourceFileName | ||
|
|
Owner
There was a problem hiding this comment.
Can you explain why we're doing this splitting and reversing?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Following up the comment in #1141:
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.