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

Provide an option to cycle through delta themes using --show-themes #550

Merged
merged 19 commits into from
Mar 29, 2021

Conversation

clnoll
Copy link
Contributor

@clnoll clnoll commented Mar 28, 2021

When delta is invoked with --show-themes, cycles through all themes in the git config, where a theme is identified as a section with a "dark" or a "light" entry. Uses the computed is_light_mode value to determine whether to show light themes or dark themes by default.

When either the --dark or --light flag is passed in with --show-themes, will additionally display the dark or light themes, respectively. When --light and --dark are passed in, shows all themes.

The user can navigate from theme to theme using the n and N navigation keys.

In addition to viewing the themes using a sample diff, users can display themes on their own diff by piping it into delta, e.g. git show | delta --show-themes.

That being said, I might still swap out the sample commits used in the display, if I find something that seems to exhibit the themes' characteristics more clearly.

I'd also like to add a light theme or two to themes.gitconfig, because right now it only has dark themes to display.

Copy link
Owner

@dandavison dandavison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fantastic! Delta should definitely have this. Some tiny comments but I can see this is pretty close.

src/cli.rs Outdated Show resolved Hide resolved
src/options/get.rs Show resolved Hide resolved
src/options/get.rs Outdated Show resolved Hide resolved
clnoll added 3 commits March 27, 2021 22:20
and give the ability to filter themes shown with --show-themes by
dark or light, using the --dark and --light flags
value to determine whether to show light or dark themes by default
@clnoll clnoll force-pushed the show-delta-themes branch from 874287b to 2e02bbe Compare March 28, 2021 02:20
let title_style = ansi_term::Style::new().bold();
let writer = output_type.handle().unwrap();

for theme in &get_themes(git_config::GitConfig::try_create()) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's display them in alphabetical order.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@@ -0,0 +1,224 @@
pub const DIFF: &[u8; 8715] = b"\
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we use one of the example diffs used in the README, or something else? I was going to say we should choose a shortish one, but I can see the attraction in using a diff long enough to not fit on the screen -- i.e. that it prevents the user from seeing two themes on the screen at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I switched to vuejs/vue@7ec4627, from the README.

Copy link
Owner

@dandavison dandavison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost there! I think just selecting the default diff is all that remains.

Copy link
Owner

@dandavison dandavison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thank you! Everything looks good. Hopefully your work here will lead to some really good themes being surfaced from delta users. (I added a link to the README section on themes to your --show-themes help text to try to help discoverability a bit more.)

@dandavison dandavison merged commit 48fe43f into dandavison:master Mar 29, 2021
@michaelblyons
Copy link

[09:37:44 ~]$ delta --version
delta 0.7.1
[09:37:53 ~]$ delta --show-themes
Pattern not found  (press RETURN)

Am I doing something wrong?

@dandavison
Copy link
Owner

Hi @michaelblyons, thank you, I don't think this is your fault. This is likely to be a bug, related to #237. Could you tell us the following information if you don't mind:

  • operating system
  • output of env | grep -i less
  • output of env | grep -i pager
  • output of less --version
  • Contents of ~/.local/share/delta/lesshst if it exists

@michaelblyons
Copy link

  • operating system

    • MacOS 11.2.3
  • output of env | grep -i less

    • Nothing (and non-zero return code)
  • output of env | grep -i pager

    • Nothing (and non-zero return code)
  • output of less --version

    • less 487 (POSIX regular expressions) (text continues)
  • Contents of ~/.local/share/delta/lesshst if it exists

    .less-history-file:
    .search
    "-b
    "alias
    "quiet
    "cert
    "-K
    "-k
    "--no
    "^Theme:
    

@dandavison
Copy link
Owner

Would you mind trying a more recent less? I'm assuming which less gives you /usr/bin/less. If you use homebrew could you do brew install less? (I'm not sure this is the problem.)

@dandavison
Copy link
Owner

Oh! I'm sorry, I know what the problem is. You need to tell delta where the themes are, like this: https://github.com/dandavison/delta#custom-color-themes

And for our part, we need to fix it so that it gives a more helpful error message!

@dandavison
Copy link
Owner

This bit is the key:

--

To use the delta themes, clone the delta repo (or download the themes.gitconfig file) and add the following entry in your gitconfig:

[include]
    path = /PATH/TO/delta/themes.gitconfig

@michaelblyons
Copy link

Oh, good. I was having some trouble with less, wherein which less suggested I was using a newly brew installed 563, but less --version still showed 487 (vs /usr/local/bin/less --version showing 563).

@dandavison
Copy link
Owner

dandavison commented Mar 30, 2021

Great, I'm glad it was nothing more serious than that. I wonder whether, in addition to that error message needing to be improved, this is pointing to moving some of the theme definitions into the executable itself. [The delta repo can continue to host a faster-evolving expanded collection, and users would still be able to create custom themes using [delta "my-theme"] either in ~/.gitconfig or in an include'd file.]

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 this pull request may close these issues.

3 participants