Skip to content

feat: print target and package names formatted as file hyperlinks #15405

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 7, 2025
Merged

feat: print target and package names formatted as file hyperlinks #15405

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions src/cargo/util/workspace.rs
View file
Open in desktop
Copy link
Member

Choose a reason for hiding this comment

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

On macOS:

  • ghostty 1.0.1 opened the default editor.
  • kitty 0.38.1 opened the default editor.
  • iterm2 3.5.11 didn't render any link, even the old existing ones. Anyway, I tried 
    printf '\e]8;;file:///Users/user/foo/Cargo.toml\e\\This is a link\e]8;;\e\\'
    It tried to scp at the first attempt as we found in feat: Make browser links out of HTML file paths #12889 (comment). However in subsequent attempts it worked correctly and opened the default editor.
  • alacritty 0.15.1 didn't render any link just like iterm2. With the same test link above it worked and opened the default editor.

@ehuss would you happen to know why iterm2 doesn't really render those links? Finished dev profile [unoptimized + debuginfo] target(s) in 0.04s should also have a rendered link, but on iterm2 I didn't see it.

Copy link
Member

Choose a reason for hiding this comment

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

I feel like this is a two-way door decision, and we already provide a way to disable the behavior, so the bar accepting this should be fairly low. However, as @ehuss has raised a concern of iterm2, I'd like to hear opinions from them :)

Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, it is disabled via

cargo/src/cargo/core/shell.rs

Line 584 in c7be060

if std::env::var_os("TERM_PROGRAM").as_deref() == Some(std::ffi::OsStr::new("iTerm.app")) {
.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah and by turning hyperlink on always, both alacritty and iTerm2 work for me. I am going to merge this since iTerm2 is already always disabled.

Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@ use anyhow::bail;
use cargo_util::paths::normalize_path;
use cargo_util::ProcessBuilder;
use std::fmt::Write;
use std::path::Path;
use std::path::PathBuf;

const ITEM_INDENT: &str = " ";

fn get_available_targets<'a>(
filter_fn: fn(&Target) -> bool,
ws: &'a Workspace<'_>,
options: &'a CompileOptions,
) -> CargoResult<Vec<&'a str>> {
) -> CargoResult<Vec<(&'a str, &'a Path)>> {
let packages = options.spec.get_packages(ws)?;

let mut targets: Vec<_> = packages
Expand All @@ -24,7 +27,12 @@ fn get_available_targets<'a>(
.iter()
.filter(|target| filter_fn(target))
})
.map(Target::name)
.map(|target| {
(
target.name(),
target.src_path().path().expect("Target is not a `Metabuild` but one of `Bin` | `Test` | `Bench` | `ExampleBin`")
)
})
.collect();

targets.sort();
Expand All @@ -48,8 +56,10 @@ fn print_available_targets(
writeln!(output, "No {} available.", plural_name)?;
} else {
writeln!(output, "Available {}:", plural_name)?;
for target in targets {
writeln!(output, " {}", target)?;
let mut shell = ws.gctx().shell();
for (name, src_path) in targets {
let link = shell.err_file_hyperlink(src_path);
writeln!(output, "{ITEM_INDENT}{link}{}{link:#}", name)?;
}
}
bail!("{}", output)
Expand All @@ -58,7 +68,7 @@ fn print_available_targets(
pub fn print_available_packages(ws: &Workspace<'_>) -> CargoResult<()> {
let packages = ws
.members()
.map(|pkg| pkg.name().as_str())
.map(|pkg| (pkg.name().as_str(), pkg.manifest_path()))
.collect::<Vec<_>>();

let mut output = "\"--package <SPEC>\" requires a SPEC format value, \
Expand All @@ -72,8 +82,10 @@ pub fn print_available_packages(ws: &Workspace<'_>) -> CargoResult<()> {
writeln!(output, "No packages available.")?;
} else {
writeln!(output, "Possible packages/workspace members:")?;
for package in packages {
writeln!(output, " {}", package)?;
let mut shell = ws.gctx().shell();
for (name, manifest_path) in packages {
let link = shell.err_file_hyperlink(manifest_path);
writeln!(output, "{ITEM_INDENT}{link}{}{link:#}", name)?;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
writeln!(output, "{ITEM_INDENT}{link}{}{link:#}", name)?;
writeln!(output, "{ITEM_INDENT}{link}{name}{link:#}")?;

}
}
bail!("{}", output)
Expand Down