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

cli: Introduce write_labeled! template formatter macro #4539

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions cli/src/command_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ use crate::merge_tools::MergeToolConfigError;
use crate::revset_util::UserRevsetEvaluationError;
use crate::template_parser::TemplateParseError;
use crate::template_parser::TemplateParseErrorKind;
use crate::templater::write_labeled;
use crate::ui::Ui;

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -775,12 +776,12 @@ fn print_error_sources(ui: &Ui, source: Option<&dyn error::Error>) -> io::Result
ui.stderr_formatter()
.with_label("error_source", |formatter| {
if err.source().is_none() {
write!(formatter.labeled("heading"), "Caused by: ")?;
write_labeled!(formatter, "heading", "Caused by: ")?;
Copy link
Contributor

Choose a reason for hiding this comment

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

I like the original code because it's clearer which string literal is a label.

writeln!(formatter, "{err}")?;
} else {
writeln!(formatter.labeled("heading"), "Caused by:")?;
for (i, err) in iter::successors(Some(err), |err| err.source()).enumerate() {
write!(formatter.labeled("heading"), "{}: ", i + 1)?;
write_labeled!(formatter, "heading", "{}: ", i + 1)?;
writeln!(formatter, "{err}")?;
}
}
Expand All @@ -791,7 +792,7 @@ fn print_error_sources(ui: &Ui, source: Option<&dyn error::Error>) -> io::Result
fn print_error_hints(ui: &Ui, hints: &[ErrorHint]) -> io::Result<()> {
for hint in hints {
ui.stderr_formatter().with_label("hint", |formatter| {
write!(formatter.labeled("heading"), "Hint: ")?;
write_labeled!(formatter, "heading", "Hint: ")?;
match hint {
ErrorHint::PlainText(message) => {
writeln!(formatter, "{message}")?;
Expand Down
5 changes: 3 additions & 2 deletions cli/src/commands/operation/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use crate::formatter::Formatter;
use crate::graphlog::get_graphlog;
use crate::graphlog::Edge;
use crate::graphlog::GraphStyle;
use crate::templater::write_labeled;
use crate::templater::TemplateRenderer;
use crate::ui::Ui;

Expand Down Expand Up @@ -384,14 +385,14 @@ fn write_modified_change_summary(
) -> Result<(), std::io::Error> {
writeln!(formatter, "Change {}", short_change_hash(change_id))?;
for commit in modified_change.added_commits.iter() {
formatter.with_label("diff", |formatter| write!(formatter.labeled("added"), "+"))?;
formatter.with_label("diff", |formatter| write_labeled!(formatter, "added", "+"))?;
Copy link
Member Author

Choose a reason for hiding this comment

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

Interesting one being nested. Could make the macro accept something like

Suggested change
formatter.with_label("diff", |formatter| write_labeled!(formatter, "added", "+"))?;
write_labeled!(formatter, "diff" & "added", "+")?;

Copy link
Contributor

Choose a reason for hiding this comment

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

label string could be separated by whitespace (and split by formatter.labeled()), but I don't know if we like it.

write!(formatter, " ")?;
commit_summary_template.format(commit, formatter)?;
writeln!(formatter)?;
}
for commit in modified_change.removed_commits.iter() {
formatter.with_label("diff", |formatter| {
write!(formatter.labeled("removed"), "-")
write_labeled!(formatter, "removed", "-")
})?;
write!(formatter, " ")?;
commit_summary_template.format(commit, formatter)?;
Expand Down
3 changes: 2 additions & 1 deletion cli/src/commands/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use crate::cli_util::CommandHelper;
use crate::command_error::CommandError;
use crate::diff_util::get_copy_records;
use crate::diff_util::DiffFormat;
use crate::templater::write_labeled;
use crate::ui::Ui;

/// Show high-level repo status
Expand Down Expand Up @@ -158,7 +159,7 @@ pub(crate) fn cmd_status(
)?;
for bookmark_name in conflicted_local_bookmarks {
write!(formatter, " ")?;
write!(formatter.labeled("bookmark"), "{bookmark_name}")?;
write_labeled!(formatter, "bookmark", "{bookmark_name}")?;
writeln!(formatter)?;
}
writeln!(
Expand Down
9 changes: 5 additions & 4 deletions cli/src/commit_templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ use crate::template_parser::TemplateDiagnostics;
use crate::template_parser::TemplateParseError;
use crate::template_parser::TemplateParseResult;
use crate::templater;
use crate::templater::write_labeled;
use crate::templater::PlainTextFormattedProperty;
use crate::templater::SizeHint;
use crate::templater::Template;
Expand Down Expand Up @@ -1043,10 +1044,10 @@ impl RefName {
// If wrapping with Rc<T> becomes common, add generic impl for Rc<T>.
impl Template for Rc<RefName> {
fn format(&self, formatter: &mut TemplateFormatter) -> io::Result<()> {
write!(formatter.labeled("name"), "{}", self.name)?;
write_labeled!(formatter, "name", "{}", self.name)?;
if let Some(remote) = &self.remote {
write!(formatter, "@")?;
write!(formatter.labeled("remote"), "{remote}")?;
write_labeled!(formatter, "remote", "{remote}")?;
}
// Don't show both conflict and unsynced sigils as conflicted ref wouldn't
// be pushed.
Expand Down Expand Up @@ -1357,8 +1358,8 @@ pub struct ShortestIdPrefix {

impl Template for ShortestIdPrefix {
fn format(&self, formatter: &mut TemplateFormatter) -> io::Result<()> {
write!(formatter.labeled("prefix"), "{}", self.prefix)?;
write!(formatter.labeled("rest"), "{}", self.rest)?;
write_labeled!(formatter, "prefix", "{}", self.prefix)?;
write_labeled!(formatter, "rest", "{}", self.rest)?;
Ok(())
}
}
Expand Down
7 changes: 4 additions & 3 deletions cli/src/diff_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ use crate::merge_tools::new_utf8_temp_dir;
use crate::merge_tools::DiffGenerateError;
use crate::merge_tools::DiffToolMode;
use crate::merge_tools::ExternalMergeTool;
use crate::templater::write_labeled;
use crate::text_util;
use crate::ui::Ui;

Expand Down Expand Up @@ -584,15 +585,15 @@ fn show_color_words_line_number(
) -> io::Result<()> {
if let Some(line_number) = left_line_number {
formatter.with_label("removed", |formatter| {
write!(formatter.labeled("line_number"), "{line_number:>4}")
write_labeled!(formatter, "line_number", "{line_number:>4}")
})?;
write!(formatter, " ")?;
} else {
write!(formatter, " ")?;
}
if let Some(line_number) = right_line_number {
formatter.with_label("added", |formatter| {
write!(formatter.labeled("line_number"), "{line_number:>4}",)
write_labeled!(formatter, "line_number", "{line_number:>4}",)
})?;
write!(formatter, ": ")?;
} else {
Expand Down Expand Up @@ -1527,7 +1528,7 @@ pub fn show_diff_stat(
stat.added + stat.removed,
if bar_added + bar_removed > 0 { " " } else { "" },
)?;
write!(formatter.labeled("added"), "{}", "+".repeat(bar_added))?;
write_labeled!(formatter, "added", "{}", "+".repeat(bar_added))?;
writeln!(formatter.labeled("removed"), "{}", "-".repeat(bar_removed))?;
}
writeln!(
Expand Down
4 changes: 3 additions & 1 deletion cli/src/formatter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ use crossterm::style::SetBackgroundColor;
use crossterm::style::SetForegroundColor;
use itertools::Itertools;

use crate::templater::write_labeled;

// Lets the caller label strings and translates the labels to colors
pub trait Formatter: Write {
/// Returns the backing `Write`. This is useful for writing data that is
Expand Down Expand Up @@ -125,7 +127,7 @@ where
pub fn write_fmt(&mut self, args: fmt::Arguments<'_>) -> io::Result<()> {
self.writer.with_labeled(|formatter| {
if let Some(heading) = self.heading.take() {
write!(formatter.labeled("heading"), "{heading}")?;
write_labeled!(formatter, "heading", "{heading}")?;
}
formatter.write_fmt(args)
})
Expand Down
5 changes: 3 additions & 2 deletions cli/src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ use crate::command_error::user_error;
use crate::command_error::CommandError;
use crate::formatter::Formatter;
use crate::progress::Progress;
use crate::templater::write_labeled;
use crate::ui::Ui;

pub fn get_git_repo(store: &Store) -> Result<git2::Repository, CommandError> {
Expand Down Expand Up @@ -381,7 +382,7 @@ impl RefStatus {
};

write!(out, "{ref_kind}")?;
write!(out.labeled("bookmark"), "{padded_ref_name}")?;
write_labeled!(out, "bookmark", "{padded_ref_name}")?;
writeln!(out, " [{import_status}] {tracking_status}")
}
}
Expand Down Expand Up @@ -412,7 +413,7 @@ pub fn print_failed_git_export(
let mut formatter = ui.stderr_formatter();
for FailedRefExport { name, reason } in failed_refs {
write!(formatter, " ")?;
write!(formatter.labeled("bookmark"), "{name}")?;
write_labeled!(formatter, "bookmark", "{name}")?;
for err in iter::successors(Some(reason as &dyn error::Error), |err| err.source()) {
write!(formatter, ": {err}")?;
}
Expand Down
16 changes: 13 additions & 3 deletions cli/src/templater.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ use crate::formatter::LabeledWriter;
use crate::formatter::PlainTextFormatter;
use crate::time_util;

macro_rules! write_labeled_ {
($formatter:expr, $label:expr, $($arg:tt)*) => {
write!(
$formatter.labeled($label),
$($arg)*
)
};
}
pub(crate) use write_labeled_ as write_labeled;

/// Represents printable type or compiled template containing placeholder value.
pub trait Template {
fn format(&self, formatter: &mut TemplateFormatter) -> io::Result<()>;
Expand Down Expand Up @@ -68,13 +78,13 @@ impl<T: Template> Template for Option<T> {

impl Template for Signature {
fn format(&self, formatter: &mut TemplateFormatter) -> io::Result<()> {
write!(formatter.labeled("name"), "{}", self.name)?;
write_labeled!(formatter, "name", "{}", self.name)?;
if !self.name.is_empty() && !self.email.is_empty() {
write!(formatter, " ")?;
}
if !self.email.is_empty() {
write!(formatter, "<")?;
write!(formatter.labeled("email"), "{}", self.email)?;
write_labeled!(formatter, "email", "{}", self.email)?;
write!(formatter, ">")?;
}
Ok(())
Expand Down Expand Up @@ -797,7 +807,7 @@ fn format_property_error_inline(
let TemplatePropertyError(err) = &err;
formatter.with_label("error", |formatter| {
write!(formatter, "<")?;
write!(formatter.labeled("heading"), "Error: ")?;
write_labeled!(formatter, "heading", "Error: ")?;
write!(formatter, "{err}")?;
for err in iter::successors(err.source(), |err| err.source()) {
write!(formatter, ": {err}")?;
Expand Down