search

phronmophobic / membrane.term

A terminal emulator in pure clojure
Eclipse Public License 1.0
53 stars 1 forks source link

Consider supporting colour schemes #1

Closed lread closed 2 years ago

lread commented 2 years ago

First, thanks for this project! It is so very cool!

Proposal As a guy wanting to use membrane.term to generate pretty screenshots for docs, I'd like to choose a pretty colour scheme.

There are boatloads of terminal colour schemes available in iTerm2-Color-Schemes in various popular formats.

Questions Not sure what format is the best to support, they all look easy to parse.

Next Steps If you like this idea, I would be happy to take a shot at a PR. I'd also be happy to step aside if you are excited by the idea of doing this work yourself. Lemme know!

phronmophobic commented 2 years ago

I love the idea of making the color scheme pluggable and the link you have seems like a really good resource. The color schemes seem like they should be pretty straightforward to integrate, but I'm not sure how or if they handle custom colors (maybe that's not even necessary).

I would be happy to accept a PR and answer any questions you might have.

lread commented 2 years ago

I've been having a wonderful time poking around with this one!

I have a few options I'm hoping you have the time and interest to review.

(Note: As a Canadian, I've been writing "colour", but rest assured, code and docs will use "color")

Option 1. Color scheme source of truth It seems that the source of truth for the iTerm2-Color-Schemes repo is, not surprisingly, in the iTerm format. I suggest we use this as our color scheme source of truth.

Option 2. Invent membrane.term format or support importing from iTerm format only The iTerm format is Apple Info.plist XML.
Readable, but wordy and arguably geeky.

The user could simply specify an iTerm format file or... we could convert the currently available set of color schemes to a more usable format.

The conversion would be invoked as warranted by you (or me or another developer) and be via a babashka script. We'd end up with one edn file per source iTerm scheme living under a resources/color-schemes directory. For example, existing Builtin Solarized Dark.itermcolors would be converted to Builtin Solarized Dark.edn.

The benefit to the user:

  1. Schemes are more accessible. The user does not have to look up a foreign repo.
  2. New schemes can be created more easily in a familiar edn format.

Command-line option might be:

:color-scheme "Solarized Dark"

For a personally created scheme:

:custom-color-scheme "my-scheme.edn"

I don't think we'd need a way to list available schemes, we could just point the user to existing examples.

An error message for a missing :color-scheme might read:

Requested :color-scheme "Perpetual Daydreams" not found.
For available color schemes, see: https://github.com/mbadolato/iTerm2-Color-Schemes#screenshots

Option 3. Our own format If option 2 is a no-go, you can stop reading. Otherwise...

Internally, membrane.term uses a rgba format (while alpha is not present in current default, it could technically be): https://github.com/phronmophobic/membrane.term/blob/a8140cd6638c0d4b5c9523c0b497028b30150456/src/com/phronemophobic/membrane/term.clj#L21-L36

I'm wondering if, for a user config file, CSS hex code values might be more approachable.

{:white "#FFFFFF" :black "#000000" ...}

I'm thinking supporting #rrggbb, #rrggbbaa, #rgb and #rgba would be sufficient. (Our conversion from option 2 would convert to the, I think more familiar, #rrggbb and when merited #rrggbbaa).

Watcha think? Those are my broad strokes and suggestions. The implementation may uncover some necessary minor tweaks.

phronmophobic commented 2 years ago

I guess there are a few questions here:

  1. What formats should membrane.term understand?
  2. Should membrane include builtin color schemes. If so, how?
  3. How should the user specify a color scheme?

1. What formats should membrane.term understand?

Membrane (the UI library) only understands colors in rgb[a] format so any format that membrane.term accepts must be converted to that rgb[a] format. Having said that, I think it's a good idea to bundle a bunch of converters to accept whatever format is convenient for the user. I think starting with an iTerm format converter is a good first step and we can add support for other format converters as requested or as we feel inclined to add converters for.

tldr; Accept as many formats as we feel like making converters to the current rgb[a] format.

I'm wondering if, for a user config file, CSS hex code values might be more approachable.

If you want to write a converter for that format, we should accept that too, but if you don't, no big deal.

2. Should membrane include builtin color schemes. If so, how?

Bundling color schemes makes sense as long as they aren't too large. To me, it makes sense to use the exact format we receive them in to make it easier to update them. Since the individual files are tiny, converting the chosen color scheme format at runtime as necessary seems reasonable. Especially since the iterm format wouldn't require any additional library dependencies to read.

3. How should the user specify a color scheme?

As long as it's mostly unambiguous, I like the idea of using a single :color-scheme cli option rather than having both :color-scheme and :custom-color-scheme. Are there any ambiguous cases between :custom-color-scheme and :color-scheme as long as we prefer custom?

Only accepting iterm format at the command line seems like a good v1. If we did accept multiple formats, it seems like we should be able to guess the format provided (eg. check the file extension) and only require an explicit :color-scheme-format if it's truly ambiguous.

:color-scheme "Solarized Dark"

For clojure's -X option, it's really annoying to write strings, eg `'"Solarized Dark"'. What do you think about also accepting keywordized versions?

lread commented 2 years ago

Thanks for your thoughtful reply, much appreciated!

  1. Cool, we'll start with only an iTerm format converter.

  2. Cool, we'll bundle copies of iTerm color schemes as is, and convert them via 1. There are currently 262 color schemes that occupy in total about 1.8mb. The schemes seem to be under the MIT license, we can attribute that in the README, I guess.

  3. Yeah, I think we can start with just :color-scheme and drop the idea of :custom-color-scheme. I find Clojure's -X cmd line syntax unnecessarily user-cruel. I'm happy to also support keywordized versions :color-scheme :solarized-dark. Another option would be to also support -M main calling and do more user-friendly cmd line processing ourselves.

One last idea to bounce off of you: Perhaps we should go much simpler for a first cut:

Happy to do whatever makes sense.

phronmophobic commented 2 years ago

There are currently 262 color schemes that occupy in total about 1.8mb.

I checked and it's only 152kb gzipped which doesn't seem too bad.

Yeah, I think we can start with just :color-scheme and drop the idea of :custom-color-scheme. I find Clojure's -X cmd line syntax unnecessarily user-cruel. I'm happy to also support keywordized versions :color-scheme :solarized-dark. Another option would be to also support -M main calling and do more user-friendly cmd line processing ourselves.

All of those seem like good options to me. Whichever you prefer.

User could download iterm color scheme files themselves and reference local file.

Maybe we could accept whatever clojure.java.io/reader accepts. That would handle files and urls (even more if being called programmatically).

Perhaps we should go much simpler for a first cut

Sounds good to me. It's easy enough to add color support first and add bundled formats later as time and energy permits.

Thanks for working on this!

lread commented 2 years ago

I think I'll start simple then. Thanks for the discussion, it really did help. Fun stuff this!

phronmophobic commented 2 years ago

It's only a nice to have, so I created a separate issue for the error message if a color scheme cannot be loaded, https://github.com/phronmophobic/membrane.term/issues/5

phronmophobic commented 2 years ago

This is so cool! I just added a few comments. Let me know what you think and I'll merge it in. Thanks!

lread commented 2 years ago

It's only a nice to have, so I created a separate issue for the error message if a color scheme cannot be loaded, #5

Oh right... tested bad XML but not missing file itself! Thanks for filing that, I'll deal with it separately.

This is so cool! I just added a few comments. Let me know what you think and I'll merge it in. Thanks!

I was expecting you'd have more "Oh Lee..." moments on my usage of membrane skia and ui. Things I wondered about:

phronmophobic commented 2 years ago

I was expecting you'd have more "Oh Lee..." moments on my usage of membrane skia and ui.

I didn't see anything weird except for I had completely forgotten that ui/fill-bordered existed. Hopefully, that means that membrane's API is at least somewhat usable 😅

is there a way to set a default background color for a window, or is drawing a rectangle like I did the way to go?

There should be! but there's not. Just created https://github.com/phronmophobic/membrane/issues/26 to track. Drawing the rectangle is currently the way to go.

is there a way to query window dimensions?

It's possible, but it uses some private functions: https://github.com/phronmophobic/membrane.term/commit/3018c632edf57ad378563fa178d45d207d0a1ebd

membrane.skia/-reshape is private because calling it with incorrect parameters can hard crash the jvm, but it should be ok if used as in the above example. Still not sure what the public interface should be. Another complication is that, ideally, it should be exposed in a consistent way for all the applicable graphics backends (eg. swing, skija, javafx, etc.)

lread commented 2 years ago

Thanks for the follow-up!

I did find the membrane API very easy to follow and usable, and the concept, 😎.