This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10765

rhg: Sort `rhg status` output correctly
ClosedPublic

Authored by SimonSapin on May 22 2021, 1:39 PM.

Details

Reviewers
pulkit
Group Reviewers
hg-reviewers
Commits
rHG62225f9da938: rhg: Sort `rhg status` output correctly
rHG83cb1700c150: rhg: Sort `rhg status` output correctly
Summary
  • The relative order of states is: modified, added, removed, deleted, unknown, ignored, clean
  • Files in the same state should be sorted by name, regardless of whether or not the were in "unsure" state based on metadata alone.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

SimonSapin created this revision.May 22 2021, 1:39 PM
pulkit accepted this revision.May 23 2021, 3:33 PM
This revision is now accepted and ready to land.May 23 2021, 3:33 PM
pulkit added a comment.May 24 2021, 8:14 AM

Needs rebase.

SimonSapin added a comment.May 24 2021, 5:30 PM

Needs rebase.

I think this is because I mixed up an amend, the test-upgrade-repo.t change should have gone into an unrelated patch. I’ve rebased anyway and https://foss.heptapod.net/octobus/mercurial-devel/-/pipelines/22305 should update the patch

baymax updated this revision to Diff 28194.May 24 2021, 5:52 PM

✅ refresh by Heptapod after a successful CI run (🐙 💚)

SimonSapin added a commit: rHG83cb1700c150: rhg: Sort `rhg status` output correctly.May 27 2021, 2:59 PM
Closed by commit rHG83cb1700c150: rhg: Sort `rhg status` output correctly (authored by SimonSapin). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/rhg/src/commands/status.rs (51 lines)

Diff 28217

rust/rhg/src/commands/status.rs

// anyway // anyway
check_exec: true, check_exec: true,
list_clean: display_states.clean, list_clean: display_states.clean,
list_unknown: display_states.unknown, list_unknown: display_states.unknown,
list_ignored: display_states.ignored, list_ignored: display_states.ignored,
collect_traversed_dirs: false, collect_traversed_dirs: false,
}; };
let ignore_file = repo.working_directory_vfs().join(".hgignore"); // TODO hardcoded let ignore_file = repo.working_directory_vfs().join(".hgignore"); // TODO hardcoded
let (ds_status, pattern_warnings) = hg::status( let (mut ds_status, pattern_warnings) = hg::status(
&dmap, &dmap,
&AlwaysMatcher, &AlwaysMatcher,
repo.working_directory_path().to_owned(), repo.working_directory_path().to_owned(),
vec![ignore_file], vec![ignore_file],
options, options,
)?; )?;
if !pattern_warnings.is_empty() { if !pattern_warnings.is_empty() {
warn!("Pattern warnings: {:?}", &pattern_warnings); warn!("Pattern warnings: {:?}", &pattern_warnings);
} }
if !ds_status.bad.is_empty() { if !ds_status.bad.is_empty() {
warn!("Bad matches {:?}", &(ds_status.bad)) warn!("Bad matches {:?}", &(ds_status.bad))
} }
if !ds_status.unsure.is_empty() { if !ds_status.unsure.is_empty() {
info!( info!(
"Files to be rechecked by retrieval from filelog: {:?}", "Files to be rechecked by retrieval from filelog: {:?}",
&ds_status.unsure &ds_status.unsure
); );
} }
// TODO check ordering to match `hg status` output. if !ds_status.unsure.is_empty()
// (this is as in `hg help status`) && (display_states.modified || display_states.clean)
if display_states.modified { {
display_status_paths(ui, &(ds_status.modified), b"M")?;
}
if !ds_status.unsure.is_empty() {
let p1: Node = parents let p1: Node = parents
.expect( .expect(
"Dirstate with no parents should not list any file to "Dirstate with no parents should not list any file to
be rechecked for modifications", be rechecked for modifications",
) )
.p1 .p1
.into(); .into();
let p1_hex = format!("{:x}", p1); let p1_hex = format!("{:x}", p1);
let mut rechecked_modified: Vec<HgPathCow> = Vec::new();
let mut rechecked_clean: Vec<HgPathCow> = Vec::new();
for to_check in ds_status.unsure { for to_check in ds_status.unsure {
if cat_file_is_modified(repo, &to_check, &p1_hex)? { if cat_file_is_modified(repo, &to_check, &p1_hex)? {
rechecked_modified.push(to_check); if display_states.modified {
ds_status.modified.push(to_check);
}
} else { } else {
rechecked_clean.push(to_check); if display_states.clean {
ds_status.clean.push(to_check);
} }
} }
if display_states.modified {
display_status_paths(ui, &rechecked_modified, b"M")?;
} }
if display_states.clean {
display_status_paths(ui, &rechecked_clean, b"C")?;
} }
if display_states.modified {
display_status_paths(ui, &mut ds_status.modified, b"M")?;
} }
if display_states.added { if display_states.added {
display_status_paths(ui, &(ds_status.added), b"A")?; display_status_paths(ui, &mut ds_status.added, b"A")?;
}
if display_states.clean {
display_status_paths(ui, &(ds_status.clean), b"C")?;
} }
if display_states.removed { if display_states.removed {
display_status_paths(ui, &(ds_status.removed), b"R")?; display_status_paths(ui, &mut ds_status.removed, b"R")?;
} }
if display_states.deleted { if display_states.deleted {
display_status_paths(ui, &(ds_status.deleted), b"!")?; display_status_paths(ui, &mut ds_status.deleted, b"!")?;
} }
if display_states.unknown { if display_states.unknown {
display_status_paths(ui, &(ds_status.unknown), b"?")?; display_status_paths(ui, &mut ds_status.unknown, b"?")?;
} }
if display_states.ignored { if display_states.ignored {
display_status_paths(ui, &(ds_status.ignored), b"I")?; display_status_paths(ui, &mut ds_status.ignored, b"I")?;
}
if display_states.clean {
display_status_paths(ui, &mut ds_status.clean, b"C")?;
} }
Ok(()) Ok(())
} }
// Probably more elegant to use a Deref or Borrow trait rather than // Probably more elegant to use a Deref or Borrow trait rather than
// harcode HgPathBuf, but probably not really useful at this point // harcode HgPathBuf, but probably not really useful at this point
fn display_status_paths( fn display_status_paths(
ui: &Ui, ui: &Ui,
paths: &[HgPathCow], paths: &mut [HgPathCow],
status_prefix: &[u8], status_prefix: &[u8],
) -> Result<(), CommandError> { ) -> Result<(), CommandError> {
paths.sort_unstable();
for path in paths { for path in paths {
// Same TODO as in commands::root // Same TODO as in commands::root
let bytes: &[u8] = path.as_bytes(); let bytes: &[u8] = path.as_bytes();
// TODO optim, probably lots of unneeded copies here, especially // TODO optim, probably lots of unneeded copies here, especially
// if out stream is buffered // if out stream is buffered
ui.write_stdout(&[status_prefix, b" ", bytes, b"\n"].concat())?; ui.write_stdout(&[status_prefix, b" ", bytes, b"\n"].concat())?;
} }
Ok(()) Ok(())