From 780d7fb59cc78591ea1d01072b63975dd69ae066 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Nov 2022 13:27:18 -0800 Subject: [PATCH] backend: rename `NormalFile` to just `File` There are no "non-normal" files, so "normal" is not needed. We have symlinks and conflicts, but they are not files, so I think just "file" is unambiguous. I left `testutils::write_normal_file()` because there it's used to mean "not executable file" (there's also a `write_executable_file()`). I left `working_copy::FileType::Normal` since renaming `Normal` there to `File` would also suggest we should rename `FileType`, and I don't know what would be a better name for that type. --- lib/src/backend.rs | 4 ++-- lib/src/conflicts.rs | 14 ++++++------- lib/src/git_backend.rs | 26 ++++++++++++------------ lib/src/local_backend.rs | 8 ++++---- lib/src/local_backend_model.rs | 32 +++++++++++++++--------------- lib/src/local_backend_model.thrift | 4 ++-- lib/src/protos/store.proto | 4 ++-- lib/src/tree.rs | 6 +++--- lib/src/working_copy.rs | 12 +++++------ lib/tests/test_conflicts.rs | 30 ++++++++++++++-------------- lib/tests/test_merge_trees.rs | 2 +- lib/tests/test_working_copy.rs | 12 +++++------ lib/testutils/src/lib.rs | 4 ++-- src/commands.rs | 12 +++++------ 14 files changed, 85 insertions(+), 85 deletions(-) diff --git a/lib/src/backend.rs b/lib/src/backend.rs index 49534cadd..a2c2a2737 100644 --- a/lib/src/backend.rs +++ b/lib/src/backend.rs @@ -322,7 +322,7 @@ pub type BackendResult = Result; #[derive(Debug, PartialEq, Eq, Clone, Hash)] pub enum TreeValue { - Normal { id: FileId, executable: bool }, + File { id: FileId, executable: bool }, Symlink(SymlinkId), Tree(TreeId), GitSubmodule(CommitId), @@ -333,7 +333,7 @@ impl ContentHash for TreeValue { fn hash(&self, state: &mut impl digest::Update) { use TreeValue::*; match *self { - Normal { ref id, executable } => { + File { ref id, executable } => { state.update(&0u32.to_le_bytes()); id.hash(state); executable.hash(state); diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index 030a25059..f51c0b9d0 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -31,13 +31,13 @@ const CONFLICT_PLUS_LINE: &[u8] = b"+++++++\n"; fn describe_conflict_part(part: &ConflictPart) -> String { match &part.value { - TreeValue::Normal { + TreeValue::File { id, executable: false, } => { format!("file with id {}", id.hex()) } - TreeValue::Normal { + TreeValue::File { id, executable: true, } => { @@ -75,7 +75,7 @@ fn file_parts(parts: &[ConflictPart]) -> Vec<&ConflictPart> { .filter(|part| { matches!( part.value, - TreeValue::Normal { + TreeValue::File { executable: false, .. } @@ -85,7 +85,7 @@ fn file_parts(parts: &[ConflictPart]) -> Vec<&ConflictPart> { } fn get_file_contents(store: &Store, path: &RepoPath, part: &ConflictPart) -> Vec { - if let TreeValue::Normal { + if let TreeValue::File { id, executable: false, } = &part.value @@ -223,7 +223,7 @@ pub fn conflict_to_materialized_value( let mut buf = vec![]; materialize_conflict(store, path, conflict, &mut buf).unwrap(); let file_id = store.write_file(path, &mut Cursor::new(&buf)).unwrap(); - TreeValue::Normal { + TreeValue::File { id: file_id, executable: false, } @@ -384,7 +384,7 @@ pub fn update_conflict_from_content( // FileIds. for (i, buf) in removed_content.iter().enumerate() { let file_id = store.write_file(path, &mut Cursor::new(buf))?; - if let TreeValue::Normal { id, executable: _ } = &mut conflict.removes[i].value { + if let TreeValue::File { id, executable: _ } = &mut conflict.removes[i].value { *id = file_id; } else { // TODO: This can actually happen. We should check earlier @@ -395,7 +395,7 @@ pub fn update_conflict_from_content( } for (i, buf) in added_content.iter().enumerate() { let file_id = store.write_file(path, &mut Cursor::new(buf))?; - if let TreeValue::Normal { id, executable: _ } = &mut conflict.adds[i].value { + if let TreeValue::File { id, executable: _ } = &mut conflict.adds[i].value { *id = file_id; } else { panic!("Found conflict markers in merge of non-files"); diff --git a/lib/src/git_backend.rs b/lib/src/git_backend.rs index 7b2177203..36980c705 100644 --- a/lib/src/git_backend.rs +++ b/lib/src/git_backend.rs @@ -252,7 +252,7 @@ impl Backend for GitBackend { } else { ( name, - TreeValue::Normal { + TreeValue::File { id, executable: false, }, @@ -263,7 +263,7 @@ impl Backend for GitBackend { let id = FileId::from_bytes(entry.id().as_bytes()); ( name, - TreeValue::Normal { + TreeValue::File { id, executable: true, }, @@ -292,11 +292,11 @@ impl Backend for GitBackend { for entry in contents.entries() { let name = entry.name().string(); let (name, id, filemode) = match entry.value() { - TreeValue::Normal { + TreeValue::File { id, executable: false, } => (name, id.as_bytes(), 0o100644), - TreeValue::Normal { + TreeValue::File { id, executable: true, } => (name, id.as_bytes(), 0o100755), @@ -483,7 +483,7 @@ fn conflict_part_from_json(json: &serde_json::Value) -> ConflictPart { fn tree_value_to_json(value: &TreeValue) -> serde_json::Value { match value { - TreeValue::Normal { id, executable } => serde_json::json!({ + TreeValue::File { id, executable } => serde_json::json!({ "file": { "id": id.hex(), "executable": executable, @@ -506,7 +506,7 @@ fn tree_value_to_json(value: &TreeValue) -> serde_json::Value { fn tree_value_from_json(json: &serde_json::Value) -> TreeValue { if let Some(json_file) = json.get("file") { - TreeValue::Normal { + TreeValue::File { id: FileId::new(bytes_vec_from_json(json_file.get("id").unwrap())), executable: json_file.get("executable").unwrap().as_bool().unwrap(), } @@ -624,14 +624,14 @@ mod tests { &TreeId::from_bytes(dir_tree_id.as_bytes()), ) .unwrap(); - let mut files = dir_tree.entries(); - let normal_file = files.next().unwrap(); - let symlink = files.next().unwrap(); - assert_eq!(files.next(), None); - assert_eq!(normal_file.name().as_str(), "normal"); + let mut entries = dir_tree.entries(); + let file = entries.next().unwrap(); + let symlink = entries.next().unwrap(); + assert_eq!(entries.next(), None); + assert_eq!(file.name().as_str(), "normal"); assert_eq!( - normal_file.value(), - &TreeValue::Normal { + file.value(), + &TreeValue::File { id: FileId::from_bytes(blob1.as_bytes()), executable: false } diff --git a/lib/src/local_backend.rs b/lib/src/local_backend.rs index 8d6e60ad6..fa4130ed9 100644 --- a/lib/src/local_backend.rs +++ b/lib/src/local_backend.rs @@ -327,9 +327,9 @@ fn tree_from_thrift(thrift_tree: &local_backend_model::Tree) -> Tree { fn tree_value_to_thrift(value: &TreeValue) -> local_backend_model::TreeValue { match value { - TreeValue::Normal { id, executable } => { - let file = local_backend_model::NormalFile::new(id.to_bytes(), *executable); - local_backend_model::TreeValue::NormalFile(file) + TreeValue::File { id, executable } => { + let file = local_backend_model::File::new(id.to_bytes(), *executable); + local_backend_model::TreeValue::File(file) } TreeValue::Symlink(id) => local_backend_model::TreeValue::SymlinkId(id.to_bytes()), TreeValue::GitSubmodule(_id) => { @@ -342,7 +342,7 @@ fn tree_value_to_thrift(value: &TreeValue) -> local_backend_model::TreeValue { fn tree_value_from_thrift(thrift_tree_value: &local_backend_model::TreeValue) -> TreeValue { match thrift_tree_value { - local_backend_model::TreeValue::NormalFile(file) => TreeValue::Normal { + local_backend_model::TreeValue::File(file) => TreeValue::File { id: FileId::from_bytes(&file.id), executable: file.executable, }, diff --git a/lib/src/local_backend_model.rs b/lib/src/local_backend_model.rs index e709cfa57..f5ddf51e0 100644 --- a/lib/src/local_backend_model.rs +++ b/lib/src/local_backend_model.rs @@ -25,26 +25,26 @@ use thrift::protocol::verify_expected_service_call; use thrift::protocol::verify_required_field_exists; // -// NormalFile +// File // #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] -pub struct NormalFile { +pub struct File { pub id: Vec, pub executable: bool, } -impl NormalFile { - pub fn new(id: Vec, executable: bool) -> NormalFile { - NormalFile { +impl File { + pub fn new(id: Vec, executable: bool) -> File { + File { id, executable, } } } -impl TSerializable for NormalFile { - fn read_from_in_protocol(i_prot: &mut dyn TInputProtocol) -> thrift::Result { +impl TSerializable for File { + fn read_from_in_protocol(i_prot: &mut dyn TInputProtocol) -> thrift::Result { i_prot.read_struct_begin()?; let mut f_1: Option> = None; let mut f_2: Option = None; @@ -70,16 +70,16 @@ impl TSerializable for NormalFile { i_prot.read_field_end()?; } i_prot.read_struct_end()?; - verify_required_field_exists("NormalFile.id", &f_1)?; - verify_required_field_exists("NormalFile.executable", &f_2)?; - let ret = NormalFile { + verify_required_field_exists("File.id", &f_1)?; + verify_required_field_exists("File.executable", &f_2)?; + let ret = File { id: f_1.expect("auto-generated code should have checked for presence of required fields"), executable: f_2.expect("auto-generated code should have checked for presence of required fields"), }; Ok(ret) } fn write_to_out_protocol(&self, o_prot: &mut dyn TOutputProtocol) -> thrift::Result<()> { - let struct_ident = TStructIdentifier::new("NormalFile"); + let struct_ident = TStructIdentifier::new("File"); o_prot.write_struct_begin(&struct_ident)?; o_prot.write_field_begin(&TFieldIdentifier::new("id", TType::String, 1))?; o_prot.write_bytes(&self.id)?; @@ -98,7 +98,7 @@ impl TSerializable for NormalFile { #[derive(Clone, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] pub enum TreeValue { - NormalFile(NormalFile), + File(File), SymlinkId(Vec), TreeId(Vec), ConflictId(Vec), @@ -117,9 +117,9 @@ impl TSerializable for TreeValue { let field_id = field_id(&field_ident)?; match field_id { 1 => { - let val = NormalFile::read_from_in_protocol(i_prot)?; + let val = File::read_from_in_protocol(i_prot)?; if ret.is_none() { - ret = Some(TreeValue::NormalFile(val)); + ret = Some(TreeValue::File(val)); } received_field_count += 1; }, @@ -178,8 +178,8 @@ impl TSerializable for TreeValue { let struct_ident = TStructIdentifier::new("TreeValue"); o_prot.write_struct_begin(&struct_ident)?; match *self { - TreeValue::NormalFile(ref f) => { - o_prot.write_field_begin(&TFieldIdentifier::new("normal_file", TType::Struct, 1))?; + TreeValue::File(ref f) => { + o_prot.write_field_begin(&TFieldIdentifier::new("file", TType::Struct, 1))?; f.write_to_out_protocol(o_prot)?; o_prot.write_field_end()?; }, diff --git a/lib/src/local_backend_model.thrift b/lib/src/local_backend_model.thrift index ed1dceb36..e75cd0960 100644 --- a/lib/src/local_backend_model.thrift +++ b/lib/src/local_backend_model.thrift @@ -12,13 +12,13 @@ // See the License for the specific language governing permissions and // limitations under the License. -struct NormalFile { +struct File { 1: required binary id, 2: required bool executable, } union TreeValue { - 1: NormalFile normal_file, + 1: File file, 2: binary symlink_id, 3: binary tree_id, 4: binary conflict_id, diff --git a/lib/src/protos/store.proto b/lib/src/protos/store.proto index 299ea197f..8f70996ac 100644 --- a/lib/src/protos/store.proto +++ b/lib/src/protos/store.proto @@ -17,13 +17,13 @@ syntax = "proto3"; package store; message TreeValue { - message NormalFile { + message File { bytes id = 1; bool executable = 2; } oneof value { - NormalFile normal_file = 2; + File file = 2; bytes symlink_id = 3; bytes tree_id = 4; bytes conflict_id = 5; diff --git a/lib/src/tree.rs b/lib/src/tree.rs index 4f1336a9f..ad0d5ccf9 100644 --- a/lib/src/tree.rs +++ b/lib/src/tree.rs @@ -633,7 +633,7 @@ fn merge_tree_value( try_resolve_file_conflict(store, &filename, &conflict)? { let id = store.write_file(&filename, &mut merged_content.as_slice())?; - Some(TreeValue::Normal { id, executable }) + Some(TreeValue::File { id, executable }) } else { let conflict_id = store.write_conflict(&filename, &conflict)?; Some(TreeValue::Conflict(conflict_id)) @@ -657,7 +657,7 @@ fn try_resolve_file_conflict( let mut added_file_ids = vec![]; for part in &conflict.removes { match &part.value { - TreeValue::Normal { id, executable } => { + TreeValue::File { id, executable } => { if *executable { exec_delta -= 1; } else { @@ -672,7 +672,7 @@ fn try_resolve_file_conflict( } for part in &conflict.adds { match &part.value { - TreeValue::Normal { id, executable } => { + TreeValue::File { id, executable } => { if *executable { exec_delta += 1; } else { diff --git a/lib/src/working_copy.rs b/lib/src/working_copy.rs index 77ab76fc5..10c9354f6 100644 --- a/lib/src/working_copy.rs +++ b/lib/src/working_copy.rs @@ -654,7 +654,7 @@ impl TreeState { match file_type { FileType::Normal { executable } => { let id = self.write_file_to_store(repo_path, disk_path)?; - Ok(TreeValue::Normal { id, executable }) + Ok(TreeValue::File { id, executable }) } FileType::Symlink => { let id = self.write_symlink_to_store(repo_path, disk_path)?; @@ -851,7 +851,7 @@ impl TreeState { } Diff::Added(after) => { let file_state = match after { - TreeValue::Normal { id, executable } => { + TreeValue::File { id, executable } => { self.write_file(&disk_path, &path, &id, executable)? } TreeValue::Symlink(id) => self.write_symlink(&disk_path, &path, &id)?, @@ -868,11 +868,11 @@ impl TreeState { stats.added_files += 1; } Diff::Modified( - TreeValue::Normal { + TreeValue::File { id: old_id, executable: old_executable, }, - TreeValue::Normal { id, executable }, + TreeValue::File { id, executable }, ) if id == old_id => { // Optimization for when only the executable bit changed assert_ne!(executable, old_executable); @@ -884,7 +884,7 @@ impl TreeState { Diff::Modified(before, after) => { fs::remove_file(&disk_path).ok(); let file_state = match (before, after) { - (_, TreeValue::Normal { id, executable }) => { + (_, TreeValue::File { id, executable }) => { self.write_file(&disk_path, &path, &id, executable)? } (_, TreeValue::Symlink(id)) => { @@ -932,7 +932,7 @@ impl TreeState { } Diff::Added(after) | Diff::Modified(_, after) => { let file_type = match after { - TreeValue::Normal { id: _, executable } => FileType::Normal { executable }, + TreeValue::File { id: _, executable } => FileType::Normal { executable }, TreeValue::Symlink(_id) => FileType::Symlink, TreeValue::Conflict(id) => FileType::Conflict { id }, TreeValue::GitSubmodule(_id) => { diff --git a/lib/tests/test_conflicts.rs b/lib/tests/test_conflicts.rs index 18e9ebaff..d8f3218fa 100644 --- a/lib/tests/test_conflicts.rs +++ b/lib/tests/test_conflicts.rs @@ -60,20 +60,20 @@ line 5 let mut conflict = Conflict { removes: vec![ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: base_id, executable: false, }, }], adds: vec![ ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: left_id, executable: false, }, }, ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: right_id, executable: false, }, @@ -158,20 +158,20 @@ line 5 let conflict = Conflict { removes: vec![ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: base_id, executable: false, }, }], adds: vec![ ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: left_id, executable: false, }, }, ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: right_id, executable: false, }, @@ -231,20 +231,20 @@ line 5 let conflict = Conflict { removes: vec![ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: base_id, executable: false, }, }], adds: vec![ ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: left_id, executable: false, }, }, ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: right_id, executable: false, }, @@ -439,20 +439,20 @@ fn test_update_conflict_from_content() { let right_file_id = testutils::write_file(store, &path, "right 1\nline 2\nright 3\n"); let conflict = Conflict { removes: vec![ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: base_file_id, executable: false, }, }], adds: vec![ ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: left_file_id, executable: false, }, }, ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: right_file_id, executable: false, }, @@ -497,20 +497,20 @@ fn test_update_conflict_from_content() { new_conflict, Conflict { removes: vec![ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: new_base_file_id, executable: false } }], adds: vec![ ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: new_left_file_id, executable: false } }, ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: new_right_file_id, executable: false } diff --git a/lib/tests/test_merge_trees.rs b/lib/tests/test_merge_trees.rs index e9bc9bae4..146c538c3 100644 --- a/lib/tests/test_merge_trees.rs +++ b/lib/tests/test_merge_trees.rs @@ -617,7 +617,7 @@ fn test_simplify_conflict_after_resolving_parent(use_git: bool) { // The conflict should now be resolved. let resolved_value = commit_c3.tree().path_value(&path); match resolved_value { - Some(TreeValue::Normal { + Some(TreeValue::File { id, executable: false, }) => { diff --git a/lib/tests/test_working_copy.rs b/lib/tests/test_working_copy.rs index e02ec0808..a48c0fc5c 100644 --- a/lib/tests/test_working_copy.rs +++ b/lib/tests/test_working_copy.rs @@ -97,21 +97,21 @@ fn test_checkout_file_transitions(use_git: bool) { } Kind::Normal => { let id = testutils::write_file(store, path, "normal file contents"); - TreeValue::Normal { + TreeValue::File { id, executable: false, } } Kind::Executable => { let id = testutils::write_file(store, path, "executable file contents"); - TreeValue::Normal { + TreeValue::File { id, executable: true, } } Kind::ExecutableNormalContent => { let id = testutils::write_file(store, path, "normal file contents"); - TreeValue::Normal { + TreeValue::File { id, executable: true, } @@ -122,20 +122,20 @@ fn test_checkout_file_transitions(use_git: bool) { let right_file_id = testutils::write_file(store, path, "right file contents"); let conflict = Conflict { removes: vec![ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: base_file_id, executable: false, }, }], adds: vec![ ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: left_file_id, executable: false, }, }, ConflictPart { - value: TreeValue::Normal { + value: TreeValue::File { id: right_file_id, executable: false, }, diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index ed1e87fc6..a8f398384 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -152,7 +152,7 @@ pub fn write_normal_file(tree_builder: &mut TreeBuilder, path: &RepoPath, conten let id = write_file(tree_builder.store(), path, contents); tree_builder.set( path.clone(), - TreeValue::Normal { + TreeValue::File { id, executable: false, }, @@ -163,7 +163,7 @@ pub fn write_executable_file(tree_builder: &mut TreeBuilder, path: &RepoPath, co let id = write_file(tree_builder.store(), path, contents); tree_builder.set( path.clone(), - TreeValue::Normal { + TreeValue::File { id, executable: true, }, diff --git a/src/commands.rs b/src/commands.rs index f64c66c9a..e2eb7c975 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -1214,7 +1214,7 @@ fn cmd_print(ui: &mut Ui, command: &CommandHelper, args: &PrintArgs) -> Result<( None => { return Err(user_error("No such path")); } - Some(TreeValue::Normal { id, .. }) => { + Some(TreeValue::File { id, .. }) => { let mut contents = repo.store().read_file(&path, &id)?; std::io::copy(&mut contents, &mut ui.stdout_formatter().as_mut())?; } @@ -1452,7 +1452,7 @@ fn diff_content( value: &TreeValue, ) -> Result, CommandError> { match value { - TreeValue::Normal { id, .. } => { + TreeValue::File { id, .. } => { let mut file_reader = repo.store().read_file(path, id).unwrap(); let mut content = vec![]; file_reader.read_to_end(&mut content)?; @@ -1482,7 +1482,7 @@ fn diff_content( fn basic_diff_file_type(value: &TreeValue) -> String { match value { - TreeValue::Normal { executable, .. } => { + TreeValue::File { executable, .. } => { if *executable { "executable file".to_string() } else { @@ -1519,11 +1519,11 @@ fn show_color_words_diff( let right_content = diff_content(repo, &path, &right_value)?; let description = match (left_value, right_value) { ( - TreeValue::Normal { + TreeValue::File { executable: left_executable, .. }, - TreeValue::Normal { + TreeValue::File { executable: right_executable, .. }, @@ -1592,7 +1592,7 @@ fn git_diff_part( let hash; let mut content = vec![]; match value { - TreeValue::Normal { id, executable } => { + TreeValue::File { id, executable } => { mode = if *executable { "100755".to_string() } else {