-
Notifications
You must be signed in to change notification settings - Fork 249
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
Print available routes as links #1674
Conversation
Signed-off-by: itowlson <[email protected]>
@@ -201,6 +209,20 @@ impl fmt::Display for RoutePattern { | |||
} | |||
} | |||
|
|||
enum Wildness { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is cute, but can we think of a clearer name? I'm not sure what it would be, but I'm pretty sure it isn't this.
@@ -129,6 +129,24 @@ macro_rules! ceprint { | |||
}; | |||
} | |||
|
|||
pub fn print_link(url: &str) { | |||
if atty::is(atty::Stream::Stdout) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a note that we should eventually move the std library support for this: https://doc.rust-lang.org/stable/std/io/trait.IsTerminal.html#tymethod.is_terminal
|
||
#[cfg(unix)] | ||
fn link(url: &str) -> String { | ||
format!("\x1B]8;;{url}\x1B\\{url}\x1B]8;;\x1B\\") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we're not using termcolor for this? Is there not support for what we're trying to achieve?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes by adding color would also help
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not an expert but can we use https://docs.rs/colored/latest/colored/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or else we can us the color as raw hexadecimal format
which also works when not want to add more dependencies
@@ -129,6 +129,24 @@ macro_rules! ceprint { | |||
}; | |||
} | |||
|
|||
pub fn print_link(url: &str) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be cleaner if this didn't directly do the printing and just returned a Cow<str>
of the escaped string so that it can be used at the callsite by a println!
macro?
Fixes #1672.
(Though actually this only addresses
spin up
- acloud-plugin
port would be needed to address @ingride's original issue.)The implementation I ended up with is not very tidy at the call site, because it needs multiple print statements for each output line. I looked at using alternate formatting to linkify routes (allowing e.g.
println!(" {id}: {url:#}{wild}");
) but I couldn't see a way to skip the formatting if the output stream wasn't a terminal. I also wondered about aterminal::print_link!
macro that would accept the whole line but only linkify the URL bit, but I couldn't see how to handle that level of generality. So, Rust whizzes, if you have any ideas, I am eager to learn!Anyway here is what the result looks like - this is WSL in Windows Terminal, other terminals may vary!
(I didn't linkify the base URL, because I was too lazy to scan the components to see if the base URL was actually mapped - I always click on a component rather than the base. But if other folks use the base then I can do it too.)