Do expect/actual for shared code in Deflater #1434
Conversation
swankjesse
changed the title
| @@ -13,8 +13,6 @@ | |||
| * See the License for the specific language governing permissions and | |||
| * limitations under the License. | |||
| */ | |||
|
|
|||
| @file:JvmName("-DeflaterSinkExtensions") | |||
There was a problem hiding this comment.
extension moved to the multiplatform zlibMain source set
| else -> null | ||
| } | ||
| zStream.avail_in = sourceByteCount.toUInt() | ||
| internal val dataProcessor: DataProcessor = object : DataProcessor() { |
There was a problem hiding this comment.
Super annoying; I can’t extend DataProcessor in Kotlin/Native because that doesn’t match the inheritance hierarchy in Kotlin/JVM.
| closed = true | ||
| actual constructor() : this(false) | ||
|
|
||
| @OptIn(UnsafeNumber::class) |
There was a problem hiding this comment.
These should be safe in practice! We don’t expose target-specific numbers in our public API
swankjesse
changed the title
| * This class does not offer any partial flush mechanism. For best performance, | ||
| * only call [flush] when application behavior requires it. | ||
| */ | ||
| expect class DeflaterSink |
There was a problem hiding this comment.
Any reason to make these public? The JVM ones have to remain public for compatibility, but these don't expose any API on top of the underlying Sink.
There was a problem hiding this comment.
This API is handy to implement web sockets, SPDY/3, and PNG. We might use it in a future multiplatform Paparazzi!
There was a problem hiding this comment.
I'm referring specifically to the class. The extension below already provides access to the functionality (although its return type should probably be just Sink).
There was a problem hiding this comment.
I’d like for it to remain public so we keep the option of having APIs like finishDeflate() public. Or APIs to supply a Dictionary mid-compression if that exotic feature is used.
| } | ||
|
|
||
| private fun Deflater(nowrap: Boolean) = Deflater(Z_BEST_COMPRESSION, nowrap) |
There was a problem hiding this comment.
No way to put this default value onto the expect so you don't need to define this function?
There was a problem hiding this comment.
Not as far as I know.
> Task :okio:compileKotlinJvm FAILED
e: file:///Volumes/Development/okio/okio/src/jvmMain/kotlin/okio/-JvmPlatform.kt:45:1 Default argument values inside expect declaration 'Deflater' are not allowed if it is actualized via typealias. Possible fix is to remove default argument values in members:
public constructor Deflater(level: Int = ..., nowrap: Boolean = ...)
No description provided.