Provide construct for command close/tear-down

[gabrielfeo]

I have a group of commands which will each one use a certain resource and must close it in the end. The current way of doing this is simply rely on each one closing in its own run method, but I feel it'd be useful to implement this once in the parent command, instead of each subcommand.

If one of them sound good, I'd be happy to contribute with a PR.

Current solution

class Reports : NoOpCliktCommand()

// subcommands of Reports:

class AnalysisA : CliktCommand() {
  override fun run() {
    startClient()
    // do work
    shutdown()
  }
}

class AnalysisB : CliktCommand() {
  override fun run() {
    startClient()
    // do work
    shutdown()
  }
}

Proposal

Clikt could check if a command is Closeable and call close() after run() ends.

class Reports : CliktCommand(), Closeable {
  override fun run() {
    startClient()
  }
  override fun close() {
    shutdown()
  }
}

// subcommands of Reports:

class AnalysisA : CliktCommand() {
  override fun run() {
    // do work
  }
}

class AnalysisB : CliktCommand() {
  override fun run() {
    // do work
  }
}

Alternatively, close could be a new open method in CliktCommand.

[ajalt]

I imagine it would be helpful to be able to register closable object on the context so that subcommands can use them. Then all a command's registered objects could be closed once all subcommands have completed.

something like

fun <T: AutoClosable> Context.use(t: T) : T

fun run() {
    currentContext.obj = currentContext.use(file.open()) // close() called automatically at end of execution
}

we'd have to use the experimental kotlin.io.AutoClosable, since java.io.Closable isn't cross platform, although we could add a jvm-only extension for Closable.

[gabrielfeo]

Great, that'd support allowMultipleSubcommands. So the general idea is this?

1. Context.use taking an AutoCloseable (regardless of being assigned to Context.obj later
2. Implementation of Context.use stores exactly 1 Closeable object
3. All commands run
4. Internally, Clikt calls close on the stored Closeable object

• What do you think of naming it Context.registerCloseable instead? I think it'd be clearer considering that Kotlin's use will close as soon as the function returns, while our function will only close much later

fun <T: AutoClosable> Context.registerCloseable(t: T) : T

fun run() {
    currentContext.obj = currentContext.registerCloseable(file.open())
}

// or when we don't need to store anything on Context.obj (my case, actually)
fun run() {
    currentContext.registerCloseable { shutdown() }
}

// And also leaves obj free to store something else
fun run() {
    currentContext.obj = aSharedObjectThatDoesntNeedClosing
    currentContext.registerCloseable { shutdown() }
}

• I couldn't find kotlin.io.AutoClosable on Kotlin 1.8 kotlin.io. Is that from a beta version? We could make this JVM-exclusive or simply don't use a type. I prefer the latter. What do you think?

fun Context.afterAllCommands(block: () -> Unit)

fun run() {
    context.afterAllCommands {
        shutdown()
        myFile.close()
    }
}

[ajalt]

AutoClosable is an experimental feature added in, i believe, 1.8.20-beta: https://youtrack.jetbrains.com/issue/KT-31066

I don't feel strongly about the name. registerClosable is probably less confusing. If we added it, it should be able to be called any number of times, and will close everything passed to it.

afterAllCommands is more flexible, but it would probably be more straight forward as an open method on CliktCommand. I don't love the idea of adding lifecycle methods, though.

[gabrielfeo]

Thanks for the link, I wasn't aware of that. Do you think we should wait until AutoCloseable is stable to implement this in Clikt or implement it as JVM-only until then?

1. Context.use taking an AutoCloseable (regardless of being assigned to Context.obj later)
2. Implementation of Context.use stores the AutoCloseable argument in an internal Collection
3. All commands run
4. Internally, Clikt calls AutoCloseable.close on each object stored in the Collection

[ajalt]

I would wait until 1.8.20 is released, but we don't have to wait until AutoClosable is stable; we can just make use have the same OpIn annotation.

[mgroth0]

I would greatly benefit from this feature. Is it ready to implement?