revset: allow resolving change id using hex digits from reverse alphabet

By separating the value spaces change ids and commit ids, we can
simplify lookup of a prefix. For example, if we know that a prefix is
for a change id, we don't have to try to find matching commit ids. I
think it might also help new users more quickly understand that change
ids are not commit ids.

This commit is a step towards that separation. It allows resolving
change ids by using hex digits from the back of the alphabet instead
of 0-f, so 'z'='0', 'y'='1', etc, and 'k'='f'. Thanks to @ilyagr for
the idea. The regular hex digits are still allowed.
This commit is contained in:
Martin von Zweigbergk 2023-02-12 10:21:29 -08:00 committed by Martin von Zweigbergk
parent b44148871a
commit 39640cc288
4 changed files with 108 additions and 1 deletions

80
lib/src/hex_util.rs Normal file
View File

@ -0,0 +1,80 @@
// Copyright 2023 The Jujutsu Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// https://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
fn to_reverse_hex_digit(b: u8) -> Option<u8> {
let value = match b {
b'0'..=b'9' => b - b'0',
b'A'..=b'F' => b - b'A' + 10,
b'a'..=b'f' => b - b'a' + 10,
_ => return None,
};
Some(b'z' - value)
}
fn to_forward_hex_digit(b: u8) -> Option<u8> {
let value = match b {
b'k'..=b'z' => b'z' - b,
b'K'..=b'Z' => b'Z' - b,
_ => return None,
};
if value < 10 {
Some(b'0' + value)
} else {
Some(b'a' + value - 10)
}
}
pub fn to_forward_hex(reverse_hex: &str) -> Option<String> {
reverse_hex
.bytes()
.map(|b| to_forward_hex_digit(b).map(char::from))
.collect()
}
pub fn to_reverse_hex(forward_hex: &str) -> Option<String> {
forward_hex
.bytes()
.map(|b| to_reverse_hex_digit(b).map(char::from))
.collect()
}
#[cfg(test)]
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_reverse_hex() {
// Empty string
assert_eq!(to_reverse_hex(""), Some("".to_string()));
assert_eq!(to_forward_hex(""), Some("".to_string()));
// Single digit
assert_eq!(to_reverse_hex("0"), Some("z".to_string()));
assert_eq!(to_forward_hex("z"), Some("0".to_string()));
// All digits
assert_eq!(
to_reverse_hex("0123456789abcdefABCDEF"),
Some("zyxwvutsrqponmlkponmlk".to_string())
);
assert_eq!(
to_forward_hex("zyxwvutsrqponmlkPONMLK"),
Some("0123456789abcdefabcdef".to_string())
);
// Invalid digit
assert_eq!(to_reverse_hex("g"), None);
assert_eq!(to_forward_hex("j"), None);
}
}

View File

@ -28,6 +28,7 @@ pub mod files;
pub mod git;
pub mod git_backend;
pub mod gitignore;
pub mod hex_util;
pub mod index;
pub mod index_store;
#[cfg(feature = "legacy-thrift")]

View File

@ -32,6 +32,7 @@ use thiserror::Error;
use crate::backend::{BackendError, BackendResult, CommitId, ObjectId};
use crate::commit::Commit;
use crate::hex_util::to_forward_hex;
use crate::index::{HexPrefix, IndexEntry, PrefixResolution};
use crate::matchers::{EverythingMatcher, Matcher, PrefixMatcher};
use crate::op_store::WorkspaceId;
@ -119,7 +120,8 @@ fn resolve_short_commit_id(
}
fn resolve_change_id(repo: RepoRef, symbol: &str) -> Result<Option<Vec<CommitId>>, RevsetError> {
if let Some(prefix) = HexPrefix::new(symbol) {
let forward_hex = to_forward_hex(symbol);
if let Some(prefix) = HexPrefix::new(forward_hex.as_deref().unwrap_or(symbol)) {
match repo.resolve_change_id_prefix(&prefix) {
PrefixResolution::NoMatch => Ok(None),
PrefixResolution::AmbiguousMatch => {

View File

@ -257,12 +257,24 @@ fn test_resolve_symbol_change_id(readonly: bool) {
"8fd68d104372910e19511df709e5dde62a548720"
)]
);
assert_eq!(
resolve_symbol(repo_ref, "zvlyx", None).unwrap(),
vec![CommitId::from_hex(
"8fd68d104372910e19511df709e5dde62a548720"
)]
);
assert_eq!(
resolve_symbol(repo_ref, "04e1c", None).unwrap(),
vec![CommitId::from_hex(
"e2ad9d861d0ee625851b8ecfcf2c727410e38720"
)]
);
assert_eq!(
resolve_symbol(repo_ref, "zvlyn", None).unwrap(),
vec![CommitId::from_hex(
"e2ad9d861d0ee625851b8ecfcf2c727410e38720"
)]
);
assert_matches!(
resolve_symbol(repo_ref, "04e1", None),
Err(RevsetError::AmbiguousIdPrefix(s)) if s == "04e1"
@ -275,6 +287,10 @@ fn test_resolve_symbol_change_id(readonly: bool) {
resolve_symbol(repo_ref, "04e13", None),
Err(RevsetError::NoSuchRevision(s)) if s == "04e13"
);
assert_matches!(
resolve_symbol(repo_ref, "zvlyw", None),
Err(RevsetError::NoSuchRevision(s)) if s == "zvlyw"
);
// Test commit/changed id conflicts.
assert_eq!(
@ -288,6 +304,14 @@ fn test_resolve_symbol_change_id(readonly: bool) {
Err(RevsetError::AmbiguousIdPrefix(s)) if s == "040"
);
// Can disambiguate by using reverse hex for change id
assert_eq!(
resolve_symbol(repo_ref, "zvz", None).unwrap(),
vec![CommitId::from_hex(
"5339432b8e7b90bd3aa1a323db71b8a5c5dcd020"
)]
);
// Test non-hex string
assert_matches!(
resolve_symbol(repo_ref, "foo", None),