Subcommand to list out available themes #48
Conversation
|
@gillespiecd Any particular reason why you closed this PR? |
|
@sharkdp After some thought, I wasn't sure if it was going to be useful or not. If you feel like it will be a good addition, then I can re-open. |
|
Didn't have the time to take a closer look at the code, but I'd certainly like to, at some point. Let's leave it open for now. Thank you for your contribution! |
|
@sharkdp Sounds good, thanks! |
gillespiecd
force-pushed
the
listthemes
branch
7 times, most recently
from
7276518 to
23f0ff4
Compare
src/main.rs
Outdated
| @@ -140,7 +169,8 @@ fn run() -> Result<()> { | |||
| SubCommand::with_name("preview") | |||
| .about("Preview a given theme") | |||
| .arg(Arg::with_name("theme").help("Name of the color theme")), | |||
| ); | |||
| ) | |||
| .subcommand(SubCommand::with_name("themes").about("Available themes for use")); | |||
There was a problem hiding this comment.
maybe: "Print a list of available themes"?
| @@ -61,6 +63,33 @@ fn load_filetypes_database(matches: &ArgMatches, user_config_path: &PathBuf) -> | |||
| } | |||
| } | |||
|
|
|||
| fn available_theme_names(user_config_path: &PathBuf) -> Result<Vec<String>> { | |||
There was a problem hiding this comment.
Shouldn't we list the user-themes in addition to the builtin themes?
There was a problem hiding this comment.
Updated, this should hopefully work now. It captures all possible theme paths.
I ran a copy to test out.
> cp themes/ayu.yml $HOME/.config/vivid/themes/ayu2.yml
> ./target/debug/vivid themes
ayu
ayu2
jellybeans
molokai
snazzy
solarized-dark
solarized-light
src/main.rs
Outdated
| .map_err(VividError::IoError)? | ||
| .file_name() | ||
| .into_string() | ||
| .map_err(|n| VividError::InvalidFileName(n.into_string().unwrap()))?; |
There was a problem hiding this comment.
If into_string has failed in the line above this one, it will also fail here inside map_err. Which, in turn, will cause .unwrap() to panic.
You can try this by running
touch ~/.config/vivid/themes/$(printf 'whatever\xFE')
before trying to list the themes.
What you could do instead (either in the error handling, or directly for listing the themes) would be to use .as_os_str().to_string_lossy() which replaces invalid UTF-8 characters with a � symbol.
There was a problem hiding this comment.
Ah yes, thanks for the detailed explanation, it did seem like bad practice to unwrap there, makes sense now.
gillespiecd
force-pushed
the
listthemes
branch
2 times, most recently
from
cdfd6dd to
cf9dc0c
Compare
Improve error handling for theme listing
gillespiecd
force-pushed
the
listthemes
branch
from
cf9dc0c to
4ac2d62
Compare
Possibly resolves #46 (not entirely sure what the ask is), or at the very least hopefully adds some useful functionality.
The goal is to display to users which themes are available to use, so that they have an idea of what can be run into
previeworgenerate.Output