Skip to content

feat: allow to exclude ABI by source path#1160

Open
Vampire wants to merge 1 commit intoautonomousapps:mainfrom
Vampire:exclude-by-source-path
Open

feat: allow to exclude ABI by source path#1160
Vampire wants to merge 1 commit intoautonomousapps:mainfrom
Vampire:exclude-by-source-path

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.

@Vampire Vampire force-pushed the exclude-by-source-path branch 2 times, most recently from 8d055f3 to 12c808b Compare April 10, 2024 22:48
@Vampire Vampire force-pushed the exclude-by-source-path branch from 12c808b to 829b357 Compare June 2, 2024 01:43
@Vampire Vampire force-pushed the exclude-by-source-path branch from 829b357 to f7e5f4d Compare July 9, 2024 10:54
@Vampire Vampire force-pushed the exclude-by-source-path branch from f7e5f4d to 5776b71 Compare August 6, 2025 17:07
@Vampire Vampire force-pushed the exclude-by-source-path branch from 5776b71 to 579967f Compare December 22, 2025 12:44
@Vampire Vampire force-pushed the exclude-by-source-path branch from 579967f to 7a282a3 Compare December 23, 2025 01:55
Copy link
Owner

@autonomousapps autonomousapps left a comment

Choose a reason for hiding this comment

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

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

Copy link
Owner

Choose a reason for hiding this comment

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

Can you explain why we're doing this splitting and reversing?

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.

2 participants