contour icon indicating copy to clipboard operation
contour copied to clipboard

Configuration changes are reported only for "gone" keys

Open whisperity opened this issue 1 year ago • 9 comments

Currently, there is no "good" way of keeping up with the changes in the project with the config file someone is using, if they have a non-standard, non-default configuration. The only method is to generate a config file and diff the two YAMLs against each other in whatever diff tool of your liking.

Contour does report configuration options that appears in the user's file but are not understood (?) by the current binary:

❯ contour
[error] Deprecated cursor config colorscheme entry. Please update your colorscheme entry for cursor.
[error] Deprecated cursor config colorscheme entry. Please update your colorscheme entry for cursor.
[error] Superfluous config key found: profiles.default.bold_is_bright
[error] Superfluous config key found: profiles.main.bold_is_bright
[error] Superfluous config key found: profiles.light.bold_is_bright
[error] Superfluous config key found: color_schemes.breeze.default.dim_foreground
[error] Superfluous config key found: color_schemes.breeze.default.bright_foreground
[error] Superfluous config key found: color_schemes.breeze.default.dim_background
[error] Superfluous config key found: color_schemes.breeze.default.bright_background
[error] Superfluous config key found: color_schemes.nightfly.default.bright_foreground

However, this does not show us in any way what new configuration options appeared. The suggestion is to add an output message (together with these, so not into the running Contour process, but the stderr of the invoking shell) with info or trace loglevel, that tells the user in an orderly fashion what keys are replaced by the default.

Unfortunately, the output of contour debug '*' is not sufficient. Note how there are empty .s at the beginning of lines, not telling which part of the tree is being parsed. Also, background_image.path exists as a top-level key, but also as the child of something else? Should logging be a top-level key, or the child of a profile?

[2022-08-06 17:41:05.464455] [config] Missing key .logging.enabled. Using default: false.
[2022-08-06 17:41:05.464481] [config] Missing key .logging.file. Using default: "".
[2022-08-06 17:41:05.464620] [config] Missing key background_image.path. Using default: "".
[2022-08-06 17:41:05.464652] [config] Missing key .background_image.opacity. Using default: 1.
[2022-08-06 17:41:05.464658] [config] Missing key .background_image.blur. Using default: false.
[2022-08-06 17:41:05.464663] [config] Missing key .background_image.path. Using default: "".
[2022-08-06 17:41:05.464690] [config] Missing key .background_image.opacity. Using default: 1.
[2022-08-06 17:41:05.464696] [config] Missing key .background_image.blur. Using default: false.
[2022-08-06 17:41:05.464700] [config] Missing key .background_image.path. Using default: "".
[2022-08-06 17:41:05.464723] [config] Missing key .background_image.opacity. Using default: 1.
[2022-08-06 17:41:05.464728] [config] Missing key .background_image.blur. Using default: false.
[2022-08-06 17:41:05.464732] [config] Missing key .background_image.path. Using default: "".

Tangential issue: useless or incomprehensible error message

Note that the first two errors do not point at anything in the config file. It does not tell you on which path the error appears. If I grep my config file for colorscheme I do not find this word, only in the comment for profiles.<profile-name>.colors:

profiles:
  master:
    # Specifies a colorscheme to use (alternatively the colors can be inlined).
    colors: "default"

Alternatively, there is a different key under colour_schemes. This is verbatim taken from the default-generated configuration.

colour_schemes:
  default:
    # Mandates the color of the cursor and potentially overridden text.
    #
    # The color can be specified in RGB as usual, plus
    # - CellForeground: Selects the cell's foreground color.
    # - CellBackground: Selects the cell's background color.
    cursor:
      # Specifies the color to be used for the actual cursor shape.
      #
      # Default: CellForeground
      default: CellForeground
      # Specifies the color to be used for the characters that would
      # be covered otherwise.
      #
      # Default: CellBackground
      text: CellBackground

whisperity avatar Aug 06 '22 15:08 whisperity

NB: The config file loading generally has a refactor long overdue. And that paired up with a possibly better config file layout, I am all in for that, and most notably: sooner is better than later.

christianparpart avatar Aug 06 '22 17:08 christianparpart

I would also like to take that opportunity to provide a more intelligent initial configuration.

  • [ ] better shortcuts based on platfrom (On mac it's usuall Meta+... whereas on Linux/Windows it's usually Ctrl+...
  • [ ] if the user has some well known but custom fonts installed (Most notable JetBrainsMono / Fira Code and patched Nerd Fonts), then chose these for the initial configuration (have a curated list of fonts to chose from).
  • [ ] if running on wayland, tell it to default to xcb platform, as we know that this is currently better than wayland (sorry).

Did I miss anything? hmm

christianparpart avatar Aug 06 '22 22:08 christianparpart

Which one is the Meta key on Mac? :grimacing:

A curated list of default fonts could be a good idea. But the important change first is to have a more meaningful schema and loader for configurations, that can emit useful error messages and make proper decisions. The fluff about what these decisions are could be added later.

Can YAML-CPP load anchors already, or are we still not there yet? :frowning:

There are a lot of outstanding issues, e.g., even if we can say that a key is missing and was substituted with the defaults completely, we cannot yet "patch" the comments in the config file to indicate to the user that new keybinds are available.

I'm starting to think that having one config file is not a good idea. Perhaps there could be multiple, one for colourschemes, one for keybinds, one for the profiles. And Contour could start by loading ALL .yamls from the config directory, loading them all into one giant config data structure?

whisperity avatar Aug 09 '22 13:08 whisperity

Command corresponds to Meta which corresponds to Windows key on non Mac keyboards

uspasojevic96 avatar Aug 09 '22 14:08 uspasojevic96

if the user has some well known but custom fonts installed

I would make this an optional, separate dependency to install these fonts. Not all users like switching between a bunch of fonts. Some have only one they like and have installed so bloating their system with other ones wouldn't be the best idea in the world. Having it optionally available for a user to choose though is a different story. That would be an explicit vice implicit install. Granted it's almost certain most known terminals probably ship with a font to ensure there's a good one to use. WT does this with their own Cascadia Code fonts for example.

WSLUser avatar Aug 15 '22 19:08 WSLUser

if the user has some well known but custom fonts installed

I would make this an optional, separate dependency to install these fonts.

Contour won't install fonts by default. It'll just query whether system has A, B, C fonts (in order) and choose the first that it finds.

Auto-installing fonts with the package, even if a separate one, could also cause legal issues...

whisperity avatar Aug 16 '22 07:08 whisperity

If you declare a font package as a dependency, that shouldn't provide legal issues. Otherwise you'd have to install all deps on a distro before installing a main application. Since we're talking packages, we'll need to ensure there's a font package in homebrew and macports. I know Chocolately and winget already have at least one: Cascadia Code. Linux obviously has plenty of font packages to choose.

WSLUser avatar Aug 16 '22 22:08 WSLUser

@WSLUser there will be no dependencies on any fonts at all. The point rather for me is what the initial configuration file should look like. We are providing a generic one that's auto-populated upon first start. All fine. And in there it'll use monospace as font family. However, most users do not even know that they can re-alias monospace to a font of their liking, so what people do is to configure their projects to use whatever font they like, manually. All fine.

With contour I would like to provide a good out-of-the-box experience, and with that comes the choice of font. While not hard-depending on any preinstalled font, we can still make it possible to scan the existing (already installed) fonts quite easily. And then - when auto-creating a fresh configuration file - we can more intelligently pick what font to configure for the first app start. There is no silver bullet that fits all use-cases and I do not even intent to remotely attempt that, but having a curated list of custom font family names and match that against the actually user-installed font names, it should be easy to figure out what font the user would "most likely" enjoy seeing. It's a gamble, I know, but just imagine the pick was wrong, because the user has two installed fonts that are also on the curated list of font-family to chose for the initial config, then it may be the one the user wants or it may not, quite likely you hit it like you may not. But just sticking with monospace in such case is even more likely wrong to begin with.

Suppose the user has 2 custom installed fonts

  • fira code
  • jetbrains mono nerd font mono

the default config currently will set font family to monospace however, which most likely is one of the more boring standard fonts having zero of the features that any of the above fonts do have (nice looking, extended PUA use, programming ligatures, ...). So whatever we might be picking from such a curated list, it'll be already a win. And all that without any additional dependency on anything.

I just felt like there's some tiny misunderstanding here, that is why I was so super-verbose now. I hope it's more clear now.

Happy sweating!

christianparpart avatar Aug 17 '22 10:08 christianparpart

Yep it's crystal clear here now thanks. I suppose that's a good idea. Do be sure to include Cascadia Code into that curated list then as it's become quite popular. To the point a Nerd Fonts custom version that adds the missing Unicode chars exists.

WSLUser avatar Aug 17 '22 13:08 WSLUser