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

Add --color flag #483

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add --color flag #483

wants to merge 2 commits into from

Conversation

sivaplaysmC
Copy link

TL;DR

Flag --color forces colored output, even if stdout is not a terminal.

Motivation

  • grep --color=always
  • ls --color=always

Application

  • Allows colored text when piping to tools like fzf outside of zk list --interactive

@sivaplaysmC
Copy link
Author

closes #482

@tjex
Copy link
Member

tjex commented Jan 3, 2025

Nice addition, although using the color flag with zk list runs the default list output template and not my own from config.toml

list = 'zk list --format "{{style \"faint\" title}} -> {{style \"blue faint\" path}} ... {{#style \"italic faint\"}}({{format-date created \"elapsed\"}}){{/style}}" $@'

This isn't a great experience as each user will be used to having the output formatted in the way they have set it.

Also, once this is sorted, it would be great if you could address the failing test (tesh) cases. Read through CONTRIBUTING.md for clarification on that. I think it's just to do with the standard output from the help menu.

@tjex
Copy link
Member

tjex commented Jan 4, 2025

So to be clearer, the color force should hook in somewhere deeper, so that it will still be applied to commands run via aliases / modified config settings.

@sivaplaysmC
Copy link
Author

the color force should hook in somewhere deeper, so that it will still be applied to commands run via aliases / modified config settings.

ok, but where do you think should the flag be parsed? i think that adding the --color as a part of the alias make more sense. Are we on the same page?

@sivaplaysmC
Copy link
Author

Nice addition, although using the color flag with zk list runs the default list output template and not my own from config.toml

list = 'zk list --format "{{style \"faint\" title}} -> {{style \"blue faint\" path}} ... {{#style \"italic faint\"}}({{format-date created \"elapsed\"}}){{/style}}" $@'

This isn't a great experience as each user will be used to having the output formatted in the way they have set it.

Also, once this is sorted, it would be great if you could address the failing test (tesh) cases. Read through CONTRIBUTING.md for clarification on that. I think it's just to do with the standard output from the help menu.

sure, i'll look into it once i complete the implementation 👍🏻

@sivaplaysmC
Copy link
Author

@tjex ,

Nice addition, although using the color flag with zk list runs the default list output template and not my own from config.toml

list = 'zk list --format "{{style \"faint\" title}} -> {{style \"blue faint\" path}} ... {{#style \"italic faint\"}}({{format-date created \"elapsed\"}}){{/style}}" $@'

This isn't a great experience as each user will be used to having the output formatted in the way they have set it.

Also, once this is sorted, it would be great if you could address the failing test (tesh) cases. Read through CONTRIBUTING.md for clarification on that. I think it's just to do with the standard output from the help menu.

The purpose of the --color flag is to display colored text, even if the ouput is not a tty (which usually happens when you are piping the output of zk to a pager or fzf).

tl;dr,
image

image

Flag `--color` forces colored output, even if stdout is not a terminal.

Motivation
- `grep --color=always`
- `ls --color=always`

Application
- Allows colored text when piping to tools like fzf outside of `zk list --interactive`
@tjex
Copy link
Member

tjex commented Jan 7, 2025

Yep, I'm on the same page.
So it would seem that this issue should only apply to calling aliases. On line 175 in main.go is the function runAlias. Zk executes an alias when it discovers that the first argument of a call to zk is in the user's aliases section.

This is why running zk --color list for me does not execute my alias.

So the question then is, do we want the --color flag to only be available to the list command? Based on the current commands we have, it makes reasonable sense (forcing the color of any of the other command outputs is not needed I would say?).

This could be the simplest solution, to just add the --color flag as a postional flag for list. Then it would simply be appended to an alias.

so zk <list-as-alias-that-gets-expanded> --color

Thoughts?

@sivaplaysmC
Copy link
Author

sivaplaysmC commented Jan 9, 2025

do we want the --color flag to only be available to the list command?

Yes. This actually makes more sense as none of the other commands use colored output (with the exception of edit -i, which uses FZF directly).

Copy link

github-actions bot commented Feb 9, 2025

This pull request has been automatically marked as stale because it has not had recent activity.

@github-actions github-actions bot added the stale No recent activity label Feb 9, 2025
@sivaplaysmC
Copy link
Author

sivaplaysmC commented Apr 7, 2025

So to be clearer, the color force should hook in somewhere deeper, so that it will still be applied to commands run via aliases / modified config settings.

Ok, I can confirm this.I just tested it out, and it does not work correctly.

export PATH=~/repos/zk:$PATH
cat .zk/config.toml | rg '\[alias' --after-context=2
zk list --color | less
zk --color list | less

The last two commands give the same output.

image
image

Do you have any idea why this happens @tjex ??

@github-actions github-actions bot removed the stale No recent activity label Apr 8, 2025
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.

2 participants