This is an archive of the discontinued Mercurial Phabricator instance.

Note to reviewers: this needs the other patches in the stack to pass all tests, but in order to make the tests in the patches actually meaningful, this had to go first.

https://foss.heptapod.net/octobus/mercurial-devel/-/jobs/243241 Successful CI run (since baymax does not need to refresh this stack)

SimonSapin added a subscriber: SimonSapin.Sep 2 2021, 2:29 PM

SimonSapin added inline comments.

rust/rhg/src/commands/cat.rs
63	What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered by tests?
63	Making a hex string just to parse it again feels a bit silly, but is likely required by the current signature of `hg::operations::cat`. That can be refactored later.

Alphare added inline comments.Sep 8 2021, 6:27 AM

rust/rhg/src/commands/cat.rs
63	What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered by tests? If `p1` is `nullrev`, it's the same path as any other revision, but if the dirstate does not exist, then it's an abort in the same way as `hg` does it. Should I add a testcase in `test-cat.t` or somewhere similar?

SimonSapin accepted this revision.Sep 9 2021, 9:41 AM

SimonSapin added inline comments.

rust/rhg/src/commands/cat.rs
63	I guess I should have just tried it: $ cd /tmp $ hg init repo $ cd repo $ hg cat a a: no such file in rev 000000000000 Indeed that look like the same code path. Still, yes adding that test sounds good.

Alphare added a commit: rHGd919b0ca8449: rhg: add support for calling `rhg cat` without a revision.Oct 1 2021, 9:21 AM

This revision was not accepted when it landed; it landed in state Needs Review.

Closed by commit rHGd919b0ca8449: rhg: add support for calling `rhg cat` without a revision (authored by Alphare). · Explain Why

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			rust/rhg/src/commands/cat.rs (43 lines)
M			tests/test-rhg.t (43 lines)

Status	Author	Revision
Closed	Alphare	D11402 rust-revset: support explicit `.` revision
Closed	Alphare	D11401 rust-revset: add separate match logic for shortcuts
Closed	Alphare	D11403 rhg: fall back if subrepos are detected
Closed	Alphare	D11381 rhg: fallback if `defaults` config is set for the current command
Closed	Alphare	D11380 rhg: fallback if the current command has any generic hook defined
Closed	Alphare	D11379 rhg-cat: fallback in presence of a fileset
Closed	Alphare	D11378 rhg-cat: fallback when detecting `.` or `..` path segments
Closed	Alphare	D11377 rhg: add support for calling `rhg cat` without a revision

Diff 30488

rust/rhg/src/commands/cat.rs

	let stripped = normalized			let stripped = normalized
	.strip_prefix(&working_directory)			.strip_prefix(&working_directory)
	// TODO: error message for path arguments outside of the repo			// TODO: error message for path arguments outside of the repo
	.map_err(\|_\| CommandError::abort(""))?;			.map_err(\|_\| CommandError::abort(""))?;
	let hg_file = HgPathBuf::try_from(stripped.to_path_buf())			let hg_file = HgPathBuf::try_from(stripped.to_path_buf())
	.map_err(\|e\| CommandError::abort(e.to_string()))?;			.map_err(\|e\| CommandError::abort(e.to_string()))?;
	files.push(hg_file);			files.push(hg_file);
	}			}
				// TODO probably move this to a util function like `repo.default_rev` or
				// something when it's used somewhere else
				let rev = match rev {
				Some(r) => r.to_string(),
				None => format!("{:x}", repo.dirstate_parents()?.p1),
				SimonSapinUnsubmitted Not Done What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered by tests? SimonSapin: What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered…
				AlphareAuthorUnsubmitted Done What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered by tests? If `p1` is `nullrev`, it's the same path as any other revision, but if the dirstate does not exist, then it's an abort in the same way as `hg` does it. Should I add a testcase in `test-cat.t` or somewhere similar? Alphare: > What should happen when p1 is null? (Such as in a new empty repository.) Is this case covered…
				SimonSapinUnsubmitted Not Done I guess I should have just tried it: $ cd /tmp $ hg init repo $ cd repo $ hg cat a a: no such file in rev 000000000000 Indeed that look like the same code path. Still, yes adding that test sounds good. SimonSapin: I guess I should have just tried it: ``` $ cd /tmp $ hg init repo $ cd repo $ hg cat a a: no…
				SimonSapinUnsubmitted Not Done Making a hex string just to parse it again feels a bit silly, but is likely required by the current signature of `hg::operations::cat`. That can be refactored later. SimonSapin: Making a hex string just to parse it again feels a bit silly, but is likely required by the…
				};

	match rev {			let output = cat(&repo, &rev, &files).map_err(\|e\| (e, rev.as_str()))?;
	Some(rev) => {
	let output = cat(&repo, rev, &files).map_err(\|e\| (e, rev))?;
	invocation.ui.write_stdout(&output.concatenated)?;			invocation.ui.write_stdout(&output.concatenated)?;
	if !output.missing.is_empty() {			if !output.missing.is_empty() {
	let short = format!("{:x}", output.node.short()).into_bytes();			let short = format!("{:x}", output.node.short()).into_bytes();
	for path in &output.missing {			for path in &output.missing {
	invocation.ui.write_stderr(&format_bytes!(			invocation.ui.write_stderr(&format_bytes!(
	b"{}: no such file in rev {}\n",			b"{}: no such file in rev {}\n",
	path.as_bytes(),			path.as_bytes(),
	short			short
	))?;			))?;
	}			}
	}			}
	if output.found_any {			if output.found_any {
	Ok(())			Ok(())
	} else {			} else {
	Err(CommandError::Unsuccessful)			Err(CommandError::Unsuccessful)
	}			}
	}			}
	None => Err(CommandError::unsupported(
	"`rhg cat` without `--rev` / `-r`",
	)),
	}
	}

tests/test-rhg.t

	Cat files			Cat files
	$ cd $TESTTMP			$ cd $TESTTMP
	$ rm -rf repository			$ rm -rf repository
	$ hg init repository			$ hg init repository
	$ cd repository			$ cd repository
	$ echo "original content" > original			$ echo "original content" > original
	$ hg add original			$ hg add original
	$ hg commit -m "add original" original			$ hg commit -m "add original" original
				Without `--rev`
				$ $NO_FALLBACK rhg cat original
				original content
				With `--rev`
	$ $NO_FALLBACK rhg cat -r 0 original			$ $NO_FALLBACK rhg cat -r 0 original
	original content			original content
	Cat copied file should not display copy metadata			Cat copied file should not display copy metadata
	$ hg copy original copy_of_original			$ hg copy original copy_of_original
	$ hg commit -m "add copy of original"			$ hg commit -m "add copy of original"
				$ $NO_FALLBACK rhg cat original
				original content
	$ $NO_FALLBACK rhg cat -r 1 copy_of_original			$ $NO_FALLBACK rhg cat -r 1 copy_of_original
	original content			original content


	Fallback to Python			Fallback to Python
	$ $NO_FALLBACK rhg cat original			$ $NO_FALLBACK rhg cat original --exclude="*.rs"
	unsupported feature: `rhg cat` without `--rev` / `-r`			unsupported feature: error: Found argument '--exclude' which wasn't expected, or isn't valid in this context

				USAGE:
				rhg cat [OPTIONS] <FILE>...

				For more information try --help

	[252]			[252]
	$ rhg cat original			$ rhg cat original --exclude="*.rs"
	original content			original content

	$ FALLBACK_EXE="$RHG_FALLBACK_EXECUTABLE"			$ FALLBACK_EXE="$RHG_FALLBACK_EXECUTABLE"
	$ unset RHG_FALLBACK_EXECUTABLE			$ unset RHG_FALLBACK_EXECUTABLE
	$ rhg cat original			$ rhg cat original --exclude="*.rs"
	abort: 'rhg.on-unsupported=fallback' without 'rhg.fallback-executable' set.			abort: 'rhg.on-unsupported=fallback' without 'rhg.fallback-executable' set.
	[255]			[255]
	$ RHG_FALLBACK_EXECUTABLE="$FALLBACK_EXE"			$ RHG_FALLBACK_EXECUTABLE="$FALLBACK_EXE"
	$ export RHG_FALLBACK_EXECUTABLE			$ export RHG_FALLBACK_EXECUTABLE

	$ rhg cat original --config rhg.fallback-executable=false			$ rhg cat original --exclude="*.rs" --config rhg.fallback-executable=false
	[1]			[1]

	$ rhg cat original --config rhg.fallback-executable=hg-non-existent			$ rhg cat original --exclude="*.rs" --config rhg.fallback-executable=hg-non-existent
	tried to fall back to a 'hg-non-existent' sub-process but got error $ENOENT$			tried to fall back to a 'hg-non-existent' sub-process but got error $ENOENT$
	unsupported feature: `rhg cat` without `--rev` / `-r`			unsupported feature: error: Found argument '--exclude' which wasn't expected, or isn't valid in this context

				USAGE:
				rhg cat [OPTIONS] <FILE>...

				For more information try --help

	[252]			[252]

	$ rhg cat original --config rhg.fallback-executable=rhg			$ rhg cat original --exclude="*.rs" --config rhg.fallback-executable=rhg
	Blocking recursive fallback. The 'rhg.fallback-executable = rhg' config points to `rhg` itself.			Blocking recursive fallback. The 'rhg.fallback-executable = rhg' config points to `rhg` itself.
	unsupported feature: `rhg cat` without `--rev` / `-r`			unsupported feature: error: Found argument '--exclude' which wasn't expected, or isn't valid in this context

				USAGE:
				rhg cat [OPTIONS] <FILE>...

				For more information try --help

	[252]			[252]

	Requirements			Requirements
	$ $NO_FALLBACK rhg debugrequirements			$ $NO_FALLBACK rhg debugrequirements
	dotencode			dotencode
	fncache			fncache
	generaldelta			generaldelta
	persistent-nodemap			persistent-nodemap

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	30167		Sep 1 2021, 12:42 PM	★	★
Diff 2	30488	rHGd919b0ca8449367977e40fd6202fb500486e83f7	Sep 1 2021, 10:13 AM	★	★

This is an archive of the discontinued Mercurial Phabricator instance.

rhg: add support for calling `rhg cat` without a revisionClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 30488

rust/rhg/src/commands/cat.rs

tests/test-rhg.t

rhg: add support for calling `rhg cat` without a revision
ClosedPublic

Revision Contents
Changeset List