Skip to content
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

clikt 4.X grew in jar size to over 8mb #507

Closed
arturbosch opened this issue Apr 7, 2024 · 8 comments · Fixed by #516
Closed

clikt 4.X grew in jar size to over 8mb #507

arturbosch opened this issue Apr 7, 2024 · 8 comments · Fixed by #516

Comments

@arturbosch
Copy link

arturbosch commented Apr 7, 2024

Hi,

first of all, like #178, I also consider this to be the most ergonomic cli parsing library out there! Thanks for making it!.

I just noticed that my bundled dependencies grew by a lot.
Some investigations led to clikt:

clikt-deps

and it's transitive dependencies like:

deps-size

As far as I understand the new markdown feature introduced in 4.0 led to the increase of dependencies.

Is it still possible for you to make the markdown (other visualization methods than just text) optional?

@ajalt
Copy link
Owner

ajalt commented Apr 7, 2024

Thanks for the kind words!

I don't have plans to make mordant optional, because I don't want to maintain a second copy of all the multiplatform terminal output, text formatting, and help formatting.

I'm curious, what does your project look like that a couple MB of package size matters?

@mgroth0
Copy link

mgroth0 commented Apr 10, 2024

Adding my vote for removing whatever dependencies are possible to remove. I make lots of small programs that use clikt so each dependency clikt has gets multiplied by a lot in my build.

@ajalt
Copy link
Owner

ajalt commented Apr 10, 2024

Mordant is Clikt's only dependency. It provides multiplatform output, prompting, text color and formatting, markdown rendering and more.

It's also part of Clikt's public API, so there's no easy way to make it optional. Even if I did, you'd have to supply your own help formatter, prompt implementation, etc. Would you willing to write all that code just to remove a single dependency?

If binary size is important, you should be build with R8 or proguard, which would eliminate all the unused code and make removing mordant mostly pointless anyway.

@mgroth0
Copy link

mgroth0 commented Apr 10, 2024

Mordant is Clikt's only dependency

Doesn't Mordant carry transitive dependencies?

Based on Mordant's build script:

  • colormath
  • markdown
  • jna

So Clikt has at least 4 dependencies for every target right?

It's also part of Clikt's public API, so there's no easy way to make it optional. Even if I did, you'd have to supply your own help formatter, prompt implementation, etc. Would you willing to write all that code just to remove a single dependency?

No, if Mordant is totally intertwined with Clikt then it is what it is. I'm not complaining, and Mordant seems really cool.

I just think that if at any point this library is at a crossroads and has a reasonable decision to make regarding adding/keeping/removing dependencies, I'd lean in the direction of keeping this minimal. If there's no easy and practical changes that can be made right now then I see no issue.

I do imagine it might be cool to see a clikt-core one day which I imagine would be a striped down version of the API. I just think that the core CliktCommand classes are really nice even if taken without some of the fancier features so I see potential for modularizing, that's all. I also think that being able to say that a project has zero dependencies is a valuable achievement in principle, but of course for some projects that will never be practical.

@ajalt
Copy link
Owner

ajalt commented Apr 10, 2024

I do minimize dependencies; Mordant is Clikt's only direct dependency, and I plan on keeping it that way. I don't even use common libraries like kotlinx.coroutines.

I've experimented with a zero-dependency core module in the past, but given how much functionality you'd lose, I'm skeptical that's there's actually a strong enough use case to justify the extra maintenance burden.

@JakeWharton
Copy link

I see two potential and separate improvements that I think could be argued for:

  1. The fastutil dep by JB markdown is egregious. I haven't looked, but I can guarantee this library is barely used and likely as premature optimization (as opposed to plain boxing collections). I would advocate for them to copy in one specialized version as needed, if needed.

  2. The markdown support baked into mordant core is a little weird. It should likely be layered above the core in a separate artifact (which still could be a mordant artifact). In the past I've talked about how the markdown support in Mordant uses private APIs to accomplish things you can't do with the public API. This is not the case today, probably, but being a separate artifact all but guarantees the APIs in Mordant core are suitable for similar downstream usage. Markdown isn't related to the goal of rich terminal rendering, but it's one opinionated format which can use it.

Now let's say both of those changes are made, there's one more argument:

Should Clikt have built-in markdown parsing? Personally I have only used this once. I think you could probably argue that the built-in help should respect double newlines but otherwise reflow text. Separately, a markdownHelp (or whatever) extension could live in clikt-markdown and depend on mordant-markdown.

Whether that happens I think is less important than the first two points. Those seems like pretty clear wins.

@mgroth0
Copy link

mgroth0 commented Apr 11, 2024

I don't want to maintain a second copy of all the multiplatform terminal output, text formatting, and help formatting.

multiplatform output, prompting, text color and formatting, markdown rendering and more.

A lot of the time I am only use Clikt as a typesafe argument parser, since it has a really nicely written API for this.

What I am gathering from this conversation is that the current dependencies are required for output.

Fancy terminal output is nice and often I want it, but it is not strictly required. Maybe a "core" module would not render output, or would only render extremely minimal simple strings.

We have been discussing in #503 about modularizing the "runner" out so that we could have a suspend runner, coroutine runner, etc. Maybe in a similar way, we can modularize out the "renderer".

Here is an idea. A ClickCommand will have a OutputRenderer:

abstract class BaseCliktCommand<T> {
    open protected val renderer: OutputRenderer = SimplePrintingRenderer
}

OutputRenderer here is an interface which com.github.ajalt.mordant.terminal.Terminal implements. But it opens the door to other types of renderers like ComposeRenderer for example.

interface OutputRenderer {
    fun info()
    fun warning()
    fun prompt()
}

Looking at the Terminal implementation I think it could look similar to that, but maybe even a bit more abstract (when we are using an OutputRenderer, we might be agnostic about whether we are outputing to a terminal, a file, a GUI, etc)

The main pro here is that a zero-dependency core becomes possible. The main con is maybe adding more code to maintain. A secondary pro is having some sort of pluggable OutputRenderer could lead to other fun ideas likeComposeRenderer, that sort of thing.

I've experimented with a zero-dependency core module in the past, but given how much functionality you'd lose, I'm skeptical that's there's actually a strong enough use case to justify the extra maintenance burden.

Are you usually maintaining this project alone? I trust your judgement about what is realistically possible.

@JakeWharton
Copy link

  1. The fastutil dep by JB markdown is egregious. I haven't looked, but I can guarantee this library is barely used and likely as premature optimization (as opposed to plain boxing collections). I would advocate for them to copy in one specialized version as needed, if needed.

😀 JetBrains/markdown#155

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 a pull request may close this issue.

4 participants