Skip to content

[fm] handle multi-line comment fields in FM displayers #10710

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
hawkw merged 5 commits into from
Jul 2, 2026
+46 −18
Merged

[fm] handle multi-line comment fields in FM displayers #10710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
87cc945
nicely handle multiline comments
hawkw Jul 1, 2026
36a35c8
put back newlines in case display
hawkw Jul 2, 2026
7de685d
remove unused `COMMENT` consts
hawkw Jul 2, 2026
f95c261
add a test for a log entry with multiline comments
hawkw Jul 2, 2026
5e058e9
Merge branch 'main' into eliza/multiline-comments
hawkw Jul 2, 2026
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
5 changes: 5 additions & 0 deletions nexus/types/output/log_entry_display_multiline_comment.out
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
multi-line comment:
// this comment
// spans multiple lines
// isn't that cool?
* flat_key: flat_value
25 changes: 22 additions & 3 deletions nexus/types/src/fm/analysis_reports.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ impl AnalysisReport {
indent,

@smklein smklein Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for pulling this into a new PR; appreciate it being decoupled

} = self;

if !comment.is_empty() {
writeln!(f, "{:indent$}// {comment}", "")?;
for line in comment.lines() {
writeln!(f, "{:indent$}// {line}", "")?;
}
writeln!(f, "{:indent$}sitrep ID: {sitrep_id}", "")?;
if cases.is_empty() {
Expand Down Expand Up @@ -205,7 +205,9 @@ impl LogEntry {
let colon = if kvs.is_empty() { "" } else { ":" };
writeln!(f, "{:indent$}{bullet}{event}{colon}", "")?;
if let Some(comment) = comment {
writeln!(f, "{:indent$} // {comment}", "")?;
for line in comment.lines() {
writeln!(f, "{:indent$} // {line}", "")?;
}
Comment on lines 205 to +210
}
for (k, v) in kvs {
fmt_json_value(f, k, v, indent + 2)?;
Expand Down Expand Up @@ -741,4 +743,21 @@ mod tests {
&output,
);
}

/// The pretty-printer handles multi-line strings in comments by outputting
/// them at the correct indentation and prefixing each line with a `//`.
#[test]
fn test_log_entry_display_multiline_comment() {
let json = serde_json::json!({
"event": "multi-line comment",
"comment": "this comment\nspans multiple lines\nisn't that cool?",
"flat_key": "flat_value",
});
let entry: LogEntry = serde_json::from_value(json).unwrap();
let output = format!("{}", entry.display_indented(0));
expectorate::assert_contents(
"output/log_entry_display_multiline_comment.out",
&output,
);
}
}
34 changes: 19 additions & 15 deletions nexus/types/src/fm/case.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ impl Metadata {
const CLOSED_IN: &str = "closed in sitrep:";
const WIDTH: usize = const_max_len(&[DE, OPENED_IN, CLOSED_IN]);

if !comment.is_empty() {
writeln!(f, "{:>indent$}// {comment}", "")?;
for line in comment.lines() {
writeln!(f, "{:>indent$}// {line}", "")?;
}
writeln!(f, "{:>indent$}{DE:<WIDTH$} {de}", "")?;
writeln!(
Expand Down Expand Up @@ -233,8 +233,8 @@ impl Fact {
};

writeln!(f, "{BULLET:>indent$}fact {id}")?;
if !comment.is_empty() {
writeln!(f, "{:>indent$}// {comment}", "")?;
for line in comment.lines() {
writeln!(f, "{:>indent$}// {line}", "")?;
}
writeln!(
f,
Expand Down Expand Up @@ -346,20 +346,21 @@ impl fmt::Display for DisplayCase<'_> {
const REPORTED_BY: &str = "reported by:";
const ADDED_IN: &str = "added in:";
const ASSIGNMENT_ID: &str = "assignment ID:";
const COMMENT: &str = "comment:";

const WIDTH: usize = const_max_len(&[
CLASS,
REPORTED_BY,
ADDED_IN,
ASSIGNMENT_ID,
COMMENT,
]);

let pn = ereport.part_number.as_deref().unwrap_or("<UNKNOWN>");
let sn =
ereport.serial_number.as_deref().unwrap_or("<UNKNOWN>");
writeln!(f, "{BULLET:>indent$}ereport {}", ereport.id)?;
for line in comment.lines() {
writeln!(f, "{:>indent$}// {line}", "")?;
}
Comment on lines 360 to +363
Comment on lines 360 to +363
writeln!(
f,
"{:>indent$}{CLASS:<WIDTH$} {}",
Expand All @@ -378,7 +379,7 @@ impl fmt::Display for DisplayCase<'_> {
this_sitrep(*assigned_sitrep_id)
)?;
writeln!(f, "{:>indent$}{ASSIGNMENT_ID:<WIDTH$} {id}", "")?;
writeln!(f, "{:>indent$}{COMMENT:<WIDTH$} {comment}\n", "",)?;

@smklein smklein Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

... I think copilot was also saying something to this effect, but do we want to add a writeln!(f, "")? here?

We took out the \n that coincidentally used to be after the comment, so now all the CaseEreports will be smushed together

@hawkw hawkw Jul 2, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

they have bullet points, so i removed the newline since i felt like it was wasting vertical space. i can put it back if you feel strongly about it?

@smklein smklein Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ah, nah, I didn't realize there was "some separator". Don't care that much

writeln!(f)?;
}
}

Expand Down Expand Up @@ -408,12 +409,13 @@ impl fmt::Display for DisplayCase<'_> {
{
const CLASS: &str = "class:";
const REQUESTED_IN: &str = "requested in:";
const COMMENT: &str = "comment:";

const WIDTH: usize =
const_max_len(&[CLASS, REQUESTED_IN, COMMENT]);
const WIDTH: usize = const_max_len(&[CLASS, REQUESTED_IN]);

writeln!(f, "{BULLET:>indent$}alert {id}",)?;
for line in comment.lines() {
writeln!(f, "{:>indent$}// {line}", "")?;
}
Comment on lines 415 to +418

@hawkw hawkw Jul 1, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

thanks i changed this on purpose

writeln!(
f,
"{:>indent$}{CLASS:<WIDTH$} {class}, v{version}",
Expand All @@ -425,7 +427,7 @@ impl fmt::Display for DisplayCase<'_> {
"",
this_sitrep(*requested_sitrep_id)
)?;
writeln!(f, "{:>indent$}{COMMENT:<WIDTH$} {comment}\n", "")?;
writeln!(f)?;
}
}

Expand All @@ -443,11 +445,13 @@ impl fmt::Display for DisplayCase<'_> {
{
const REQUESTED_IN: &str = "requested in:";
const DATA: &str = "data:";
const COMMENT: &str = "comment:";
const WIDTH: usize =
const_max_len(&[REQUESTED_IN, DATA, COMMENT]);
const WIDTH: usize = const_max_len(&[REQUESTED_IN, DATA]);

writeln!(f, "{BULLET:>indent$}bundle {id}",)?;

for line in comment.lines() {
writeln!(f, "{:>indent$}// {line}", "")?;
}
Comment on lines 450 to +454
writeln!(
f,
"{:>indent$}{REQUESTED_IN:<WIDTH$} {requested_sitrep_id}{}",
Expand All @@ -456,7 +460,7 @@ impl fmt::Display for DisplayCase<'_> {
)?;
writeln!(f, "{:>indent$}{DATA}", "")?;
writeln!(f, "{}", data_selection.display(indent + 2))?;
writeln!(f, "{:>indent$}{COMMENT:<WIDTH$} {comment}\n", "")?;

@smklein smklein Jul 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think copilot is weird with where it's choosing to insert commentary , but the \n that was removed was here. I think I agree; it's worth putting it back?

@hawkw hawkw Jul 2, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ok, i will return the newlines

writeln!(f)?;
}
}

Expand Down
Loading