search

Kotlin / binary-compatibility-validator

Public API management tool
Apache License 2.0
821 stars 59 forks source link

Klib `.api` files could use some vertical spacing #196

Closed JakeWharton closed 4 months ago

JakeWharton commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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]
dovchinnikov commented 7 months ago

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: