Skip to content

Klib .api files could use some vertical spacing #196

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

Closed
JakeWharton opened this issue Mar 20, 2024 · 4 comments · Fixed by #225
Closed

Klib .api files could use some vertical spacing #196

JakeWharton opened this issue Mar 20, 2024 · 4 comments · Fixed by #225
Assignees
Labels
enhancement New feature or request klib

Comments

@JakeWharton
Copy link

JakeWharton commented Mar 20, 2024

This will make them visually easier to parse, but should also aid in diffing.

Examples:

Top-level types should be separated by a single vertical whitespace

 abstract fun interface app.cash.redwood.protocol/ChangesSink { // app.cash.redwood.protocol/ChangesSink|null[0]
     abstract fun sendChanges(kotlin.collections/List<app.cash.redwood.protocol/Change>) // app.cash.redwood.protocol/ChangesSink.sendChanges|sendChanges(kotlin.collections.List<app.cash.redwood.protocol.Change>){}[0]
 }
+
 abstract fun interface app.cash.redwood.protocol/EventSink { // app.cash.redwood.protocol/EventSink|null[0]
     abstract fun sendEvent(app.cash.redwood.protocol/Event) // app.cash.redwood.protocol/EventSink.sendEvent|sendEvent(app.cash.redwood.protocol.Event){}[0]
 }

Nested types should be prefixed with a single vertical whitespace

 final class app.cash.redwood.protocol/Create : app.cash.redwood.protocol/Change { // app.cash.redwood.protocol/Create|null[0]
     constructor <init>(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/WidgetTag) // app.cash.redwood.protocol/Create.<init>|<init>(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.WidgetTag){}[0]
     final fun equals(kotlin/Any?): kotlin/Boolean // app.cash.redwood.protocol/Create.equals|equals(kotlin.Any?){}[0]
     final fun hashCode(): kotlin/Int // app.cash.redwood.protocol/Create.hashCode|hashCode(){}[0]
     final fun toString(): kotlin/String // app.cash.redwood.protocol/Create.toString|toString(){}[0]
+
     final object $serializer : kotlinx.serialization.internal/GeneratedSerializer<app.cash.redwood.protocol/Create> { // app.cash.redwood.protocol/Create.$serializer|null[0]
         final fun childSerializers(): kotlin/Array<kotlinx.serialization/KSerializer<*>> // app.cash.redwood.protocol/Create.$serializer.childSerializers|childSerializers(){}[0]
         final fun deserialize(kotlinx.serialization.encoding/Decoder): app.cash.redwood.protocol/Create // app.cash.redwood.protocol/Create.$serializer.deserialize|deserialize(kotlinx.serialization.encoding.Decoder){}[0]
         final fun serialize(kotlinx.serialization.encoding/Encoder, app.cash.redwood.protocol/Create) // app.cash.redwood.protocol/Create.$serializer.serialize|serialize(kotlinx.serialization.encoding.Encoder;app.cash.redwood.protocol.Create){}[0]
         final val descriptor // app.cash.redwood.protocol/Create.$serializer.descriptor|{}descriptor[0]
             final fun <get-descriptor>(): kotlinx.serialization.descriptors/SerialDescriptor // app.cash.redwood.protocol/Create.$serializer.descriptor.<get-descriptor>|<get-descriptor>(){}[0]
     }
+
     final object Companion { // app.cash.redwood.protocol/Create.Companion|null[0]
         final fun serializer(): kotlinx.serialization/KSerializer<app.cash.redwood.protocol/Create> // app.cash.redwood.protocol/Create.Companion.serializer|serializer(){}[0]
     }

Target headers should be separated with a single vertical whitespace

 }
+
 // Targets: [ios]
 final class app.cash.redwood.widget/UIViewChildren : app.cash.redwood.widget/Widget.Children<platform.UIKit/UIView> { // app.cash.redwood.widget/UIViewChildren|null[0]
     constructor <init>(platform.UIKit/UIView, kotlin/Function2<platform.UIKit/UIView, kotlin/Int, kotlin/Unit> =..., kotlin/Function2<kotlin/Int, kotlin/Int, kotlin/Array<platform.UIKit/UIView>> =...) // app.cash.redwood.widget/UIViewChildren.<init>|<init>(platform.UIKit.UIView;kotlin.Function2<platform.UIKit.UIView,kotlin.Int,kotlin.Unit>;kotlin.Function2<kotlin.Int,kotlin.Int,kotlin.Array<platform.UIKit.UIView>>){}[0]
     final fun insert(kotlin/Int, app.cash.redwood.widget/Widget<platform.UIKit/UIView>) // app.cash.redwood.widget/UIViewChildren.insert|insert(kotlin.Int;app.cash.redwood.widget.Widget<platform.UIKit.UIView>){}[0]
     final fun move(kotlin/Int, kotlin/Int, kotlin/Int) // app.cash.redwood.widget/UIViewChildren.move|move(kotlin.Int;kotlin.Int;kotlin.Int){}[0]
     final fun onModifierUpdated(kotlin/Int, app.cash.redwood.widget/Widget<platform.UIKit/UIView>) // app.cash.redwood.widget/UIViewChildren.onModifierUpdated|onModifierUpdated(kotlin.Int;app.cash.redwood.widget.Widget<platform.UIKit.UIView>){}[0]
     final fun remove(kotlin/Int, kotlin/Int) // app.cash.redwood.widget/UIViewChildren.remove|remove(kotlin.Int;kotlin.Int){}[0]
     final val widgets // app.cash.redwood.widget/UIViewChildren.widgets|{}widgets[0]
         final fun <get-widgets>(): kotlin.collections/List<app.cash.redwood.widget/Widget<platform.UIKit/UIView>> // app.cash.redwood.widget/UIViewChildren.widgets.<get-widgets>|<get-widgets>(){}[0]
}
+
 // Targets: [ios]
 open class app.cash.redwood.widget/RedwoodUIView : app.cash.redwood.widget/RedwoodView<platform.UIKit/UIView> { // app.cash.redwood.widget/RedwoodUIView|null[0]
@JakeWharton
Copy link
Author

It might be nice to separate constructors, properties, and functions, too.

final class app.cash.redwood.protocol/Event { // app.cash.redwood.protocol/Event|null[0]
    constructor <init>(app.cash.redwood.protocol/Id, app.cash.redwood.protocol/EventTag, kotlin.collections/List<kotlinx.serialization.json/JsonElement> =...) // app.cash.redwood.protocol/Event.<init>|<init>(app.cash.redwood.protocol.Id;app.cash.redwood.protocol.EventTag;kotlin.collections.List<kotlinx.serialization.json.JsonElement>){}[0]

    final val args // app.cash.redwood.protocol/Event.args|{}args[0]
        final fun <get-args>(): kotlin.collections/List<kotlinx.serialization.json/JsonElement> // app.cash.redwood.protocol/Event.args.<get-args>|<get-args>(){}[0]
    final val id // app.cash.redwood.protocol/Event.id|{}id[0]
        final fun <get-id>(): app.cash.redwood.protocol/Id // app.cash.redwood.protocol/Event.id.<get-id>|<get-id>(){}[0]
    final val tag // app.cash.redwood.protocol/Event.tag|{}tag[0]
        final fun <get-tag>(): app.cash.redwood.protocol/EventTag // app.cash.redwood.protocol/Event.tag.<get-tag>|<get-tag>(){}[0]

    final fun equals(kotlin/Any?): kotlin/Boolean // app.cash.redwood.protocol/Event.equals|equals(kotlin.Any?){}[0]
    final fun hashCode(): kotlin/Int // app.cash.redwood.protocol/Event.hashCode|hashCode(){}[0]
    final fun toString(): kotlin/String // app.cash.redwood.protocol/Event.toString|toString(){}[0]
}

@fzhinkin fzhinkin added the klib label Mar 20, 2024
@fzhinkin fzhinkin self-assigned this Mar 20, 2024
@fzhinkin fzhinkin added the enhancement New feature or request label Mar 20, 2024
@fzhinkin
Copy link
Collaborator

Target headers should be separated with a single vertical whitespace

Wouldn't it be to "noisy" for a stride of single-line declarations? Like here:

+
// Targets: [js]
final const val app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsArray$stableprop // app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsArray$stableprop|#static{}app_cash_redwood_protocol_guest_JsArray$stableprop[0]
+
// Targets: [js]
final const val app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsArrayList$stableprop // app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsArrayList$stableprop|#static{}app_cash_redwood_protocol_guest_JsArrayList$stableprop[0]
+
// Targets: [js]
final const val app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsMap$stableprop // app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsMap$stableprop|#static{}app_cash_redwood_protocol_guest_JsMap$stableprop[0]

@JakeWharton
Copy link
Author

Initially I didn't actually realize these weren't sections but are instead headers on a single declarations. I edited the original from "Target group" to "Target header" after the fact.

I do think it's better spaced out than not, even with the target comment.

This file is text-based, so it's for humans on some level as well as the tool and visually I think that's much easier to parse. Especially given the lines are so long. You either need to correctly track which line you're on when you scan to the next one (which is proven harder as the lines get longer) or they're going to be wrapped and the separation will be nice.

Plus you can make git-diff prefer breaking on empty lines. I believe if you add a new function, without adding spaces, you'll find the diff to look like this:

 // Targets: [js]
 final const val app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsArray$stableprop // app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsArray$stableprop|#static{}app_cash_redwood_protocol_guest_JsArray$stableprop[0]
 // Targets: [js]
+final const val app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsArrayList$stableprop // app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsArrayList$stableprop|#static{}app_cash_redwood_protocol_guest_JsArrayList$stableprop[0]
+// Targets: [js]
 final const val app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsMap$stableprop // app.cash.redwood.protocol.guest/app_cash_redwood_protocol_guest_JsMap$stableprop|#static{}app_cash_redwood_protocol_guest_JsMap$stableprop[0]

@fzhinkin fzhinkin added this to the klib/0.15.0-Beta.3 milestone Mar 26, 2024
@dovchinnikov
Copy link

Example from IJ https://github.com/JetBrains/intellij-community/blob/master/platform/util/diff/api-dump.txt

Format:

modifiers:com/foo/bar/ClassName
- com/foo/baz/Supertype1
- com/foo/Supertype2
- ...
- modifiers:name:descriptor
- modifiers:name:descriptor
- ...

Modifiers:

  • * - marked with @ApiStatus.Experimental.
  • b - synthetic (as in binary visible). Members with both ACC_SYNTHETIC and ACC_BRIDGE are not included in the dump.
  • p - protected. Nothing for public because it's the default. private and package-private members are not included.
  • s - static.
  • @ - annotation.
  • e - enum.
  • f - final.
  • a - abstract.
  • c - class. Nothing for interface because it should be the default.

@fzhinkin fzhinkin linked a pull request May 6, 2024 that will close this issue
ALikhachev added a commit to Kotlin/kotlinx.serialization that referenced this issue Jul 23, 2024
This is required to ensure compatibility of the build with Kotlin 2.1+ after resolving https://youtrack.jetbrains.com/issue/KT-61706
Adds kotlinx-serialization-json-tests to ignored projects: Kotlin/binary-compatibility-validator#243
Reorders entries in dumps: Kotlin/binary-compatibility-validator#225, Kotlin/binary-compatibility-validator#196
The entries are only reordered, but contain no significant changes.
ALikhachev added a commit to Kotlin/kotlinx-benchmark that referenced this issue Jul 23, 2024
Also, update the dump to reflect the actual state of the Gradle plugin.
Reorders entries in the dumps: Kotlin/binary-compatibility-validator#225, Kotlin/binary-compatibility-validator#196
Removes internal constants from the dump: Kotlin/binary-compatibility-validator#90
Fixes #248
sandwwraith pushed a commit to Kotlin/kotlinx.serialization that referenced this issue Jul 23, 2024
This is required to ensure compatibility of the build with Kotlin 2.1+ after resolving https://youtrack.jetbrains.com/issue/KT-61706
Adds kotlinx-serialization-json-tests to ignored projects: Kotlin/binary-compatibility-validator#243
Reorders entries in dumps: Kotlin/binary-compatibility-validator#225, Kotlin/binary-compatibility-validator#196
The entries are only reordered, but contain no significant changes.
sandwwraith pushed a commit to Kotlin/kotlinx.serialization that referenced this issue Aug 1, 2024
This is required to ensure compatibility of the build with Kotlin 2.1+ after resolving https://youtrack.jetbrains.com/issue/KT-61706
Adds kotlinx-serialization-json-tests to ignored projects: Kotlin/binary-compatibility-validator#243
Reorders entries in dumps: Kotlin/binary-compatibility-validator#225, Kotlin/binary-compatibility-validator#196
The entries are only reordered, but contain no significant changes.

Cherry-picked from 46f406d
sandwwraith added a commit to Kotlin/kotlinx.serialization that referenced this issue Aug 5, 2024
This is required to ensure compatibility of the build with Kotlin 2.1+ after resolving https://youtrack.jetbrains.com/issue/KT-61706
Adds kotlinx-serialization-json-tests to ignored projects: Kotlin/binary-compatibility-validator#243
Reorders entries in dumps: Kotlin/binary-compatibility-validator#225, Kotlin/binary-compatibility-validator#196
The entries are only reordered, but contain no significant changes.

Cherry-picked from 46f406d

Co-authored-by: Alexander Likhachev <alikhachev@users.noreply.github.com>
woainikk pushed a commit to Kotlin/kotlinx.serialization that referenced this issue Aug 7, 2024
This is required to ensure compatibility of the build with Kotlin 2.1+ after resolving https://youtrack.jetbrains.com/issue/KT-61706
Adds kotlinx-serialization-json-tests to ignored projects: Kotlin/binary-compatibility-validator#243
Reorders entries in dumps: Kotlin/binary-compatibility-validator#225, Kotlin/binary-compatibility-validator#196
The entries are only reordered, but contain no significant changes.
woainikk added a commit to Kotlin/kotlinx.serialization that referenced this issue Aug 7, 2024
This is required to ensure compatibility of the build with Kotlin 2.1+ after resolving https://youtrack.jetbrains.com/issue/KT-61706
Adds kotlinx-serialization-json-tests to ignored projects: Kotlin/binary-compatibility-validator#243
Reorders entries in dumps: Kotlin/binary-compatibility-validator#225, Kotlin/binary-compatibility-validator#196
The entries are only reordered, but contain no significant changes.
qurbonzoda pushed a commit to Kotlin/kotlinx-benchmark that referenced this issue Aug 15, 2024
Also, update the dump to reflect the actual state of the Gradle plugin.
Reorders entries in the dumps: Kotlin/binary-compatibility-validator#225, Kotlin/binary-compatibility-validator#196
Removes internal constants from the dump: Kotlin/binary-compatibility-validator#90
Fixes #248
woainikk pushed a commit to Kotlin/kotlinx.serialization that referenced this issue Aug 26, 2024
This is required to ensure compatibility of the build with Kotlin 2.1+ after resolving https://youtrack.jetbrains.com/issue/KT-61706
Adds kotlinx-serialization-json-tests to ignored projects: Kotlin/binary-compatibility-validator#243
Reorders entries in dumps: Kotlin/binary-compatibility-validator#225, Kotlin/binary-compatibility-validator#196
The entries are only reordered, but contain no significant changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request klib
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants