From d657572dfd1be342da9bdc2c5da1db352e37773f Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Thu, 17 Nov 2022 17:21:31 +0000 Subject: [PATCH] radicle-surf: remove file_system::path The file_system types, Label & Path, are overengineered. It's more natural to use the Path and PathBuf types provided by the std library. This replaces any mention of Label with String and any use of the old Path with either std::path::Path or std::path::PathBuf, depending on what kind of ownwership is needed. Anywhere a "root" directory is needed `PathBuf::new()` is used, i.e. an empty `PathBuf`. Otherwise `Path::new` can used to pass directory, or in fact a &str can also be used since it implements AsRef. The `File` and `Directory` types also contain there relative paths, which can be retrieved while traversing the Tree. Note the special file name `"special/faux\\path"` requires the use of an escaping string construction, i.e. `r"special/faux\\path"`. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- radicle-surf/examples/diff.rs | 6 +- radicle-surf/src/commit.rs | 9 +- radicle-surf/src/diff.rs | 30 +- radicle-surf/src/diff/git.rs | 43 +- radicle-surf/src/file_system.rs | 5 - radicle-surf/src/file_system/directory.rs | 186 +++++--- radicle-surf/src/file_system/error.rs | 79 ---- radicle-surf/src/file_system/path.rs | 453 ------------------- radicle-surf/src/file_system/path/unsound.rs | 62 --- radicle-surf/src/git/commit.rs | 2 +- radicle-surf/src/git/history.rs | 20 +- radicle-surf/src/git/repo.rs | 62 +-- radicle-surf/src/lib.rs | 3 - radicle-surf/src/nonempty.rs | 33 -- radicle-surf/src/object.rs | 14 +- radicle-surf/src/object/blob.rs | 46 +- radicle-surf/src/object/tree.rs | 102 ++--- radicle-surf/t/src/file_system.rs | 25 - radicle-surf/t/src/git.rs | 48 +- radicle-surf/t/src/lib.rs | 3 - 20 files changed, 314 insertions(+), 917 deletions(-) delete mode 100644 radicle-surf/src/file_system/error.rs delete mode 100644 radicle-surf/src/file_system/path.rs delete mode 100644 radicle-surf/src/file_system/path/unsound.rs delete mode 100644 radicle-surf/src/nonempty.rs diff --git a/radicle-surf/examples/diff.rs b/radicle-surf/examples/diff.rs index 511310b..8c7fb33 100644 --- a/radicle-surf/examples/diff.rs +++ b/radicle-surf/examples/diff.rs @@ -58,13 +58,13 @@ fn init_repository_or_exit(path_to_repo: &str) -> git::Repository { fn print_diff_summary(diff: &Diff, elapsed_nanos: u128) { diff.created.iter().for_each(|created| { - println!("+++ {}", created.path); + println!("+++ {:?}", created.path); }); diff.deleted.iter().for_each(|deleted| { - println!("--- {}", deleted.path); + println!("--- {:?}", deleted.path); }); diff.modified.iter().for_each(|modified| { - println!("mod {}", modified.path); + println!("mod {:?}", modified.path); }); println!( diff --git a/radicle-surf/src/commit.rs b/radicle-surf/src/commit.rs index 7318e00..a5d39b8 100644 --- a/radicle-surf/src/commit.rs +++ b/radicle-surf/src/commit.rs @@ -17,6 +17,8 @@ //! Represents a commit. +use std::path::PathBuf; + use git_ref_format::RefString; #[cfg(feature = "serialize")] use serde::{ @@ -26,7 +28,6 @@ use serde::{ use crate::{ diff, - file_system, git::{self, glob, Glob, Repository}, person::Person, revision::Revision, @@ -245,10 +246,6 @@ pub fn commits(repo: &Repository, maybe_revision: Option) -> Result, } @@ -309,7 +309,7 @@ impl Diff { pub(crate) fn add_modified_file( &mut self, - path: Path, + path: PathBuf, hunks: impl Into, eof: Option, ) { @@ -325,15 +325,15 @@ impl Diff { }); } - pub(crate) fn add_moved_file(&mut self, old_path: Path, new_path: Path) { + pub(crate) fn add_moved_file(&mut self, old_path: PathBuf, new_path: PathBuf) { self.moved.push(MoveFile { old_path, new_path }); } - pub(crate) fn add_copied_file(&mut self, old_path: Path, new_path: Path) { + pub(crate) fn add_copied_file(&mut self, old_path: PathBuf, new_path: PathBuf) { self.copied.push(CopyFile { old_path, new_path }); } - pub(crate) fn add_modified_binary_file(&mut self, path: Path) { + pub(crate) fn add_modified_binary_file(&mut self, path: PathBuf) { self.modified.push(ModifiedFile { path, diff: FileDiff::Binary, @@ -341,11 +341,11 @@ impl Diff { }); } - pub(crate) fn add_created_file(&mut self, path: Path, diff: FileDiff) { + pub(crate) fn add_created_file(&mut self, path: PathBuf, diff: FileDiff) { self.created.push(CreateFile { path, diff }); } - pub(crate) fn add_deleted_file(&mut self, path: Path, diff: FileDiff) { + pub(crate) fn add_deleted_file(&mut self, path: PathBuf, diff: FileDiff) { self.deleted.push(DeleteFile { path, diff }); } diff --git a/radicle-surf/src/diff/git.rs b/radicle-surf/src/diff/git.rs index d4d6831..1c00507 100644 --- a/radicle-surf/src/diff/git.rs +++ b/radicle-surf/src/diff/git.rs @@ -17,15 +17,12 @@ use std::convert::TryFrom; -use crate::{ - diff::{self, Diff, EofNewLine, Hunk, Hunks, Line, LineDiff}, - file_system::Path, -}; +use crate::diff::{self, Diff, EofNewLine, Hunk, Hunks, Line, LineDiff}; pub mod error { - use thiserror::Error; + use std::path::PathBuf; - use crate::file_system::{self, Path}; + use thiserror::Error; #[derive(Debug, Error, PartialEq, Eq)] #[non_exhaustive] @@ -54,8 +51,6 @@ pub mod error { #[error("git delta type is not handled")] DeltaUnhandled(git2::Delta), #[error(transparent)] - FileSystem(#[from] file_system::Error), - #[error(transparent)] Git(#[from] git2::Error), #[error(transparent)] Hunk(#[from] Hunk), @@ -63,7 +58,7 @@ pub mod error { Line(#[from] LineDiff), /// A patch is unavailable. #[error("couldn't retrieve patch for {0}")] - PatchUnavailable(Path), + PatchUnavailable(PathBuf), /// A The path of a file isn't available. #[error("couldn't retrieve file path")] PathUnavailable, @@ -95,8 +90,10 @@ impl<'a> TryFrom> for Diff { match delta.status() { Delta::Added => { let diff_file = delta.new_file(); - let path = diff_file.path().ok_or(error::Diff::PathUnavailable)?; - let path = Path::try_from(path.to_path_buf())?; + let path = diff_file + .path() + .ok_or(error::Diff::PathUnavailable)? + .to_path_buf(); let patch = Patch::from_diff(&git_diff, idx)?; if let Some(patch) = patch { @@ -117,9 +114,10 @@ impl<'a> TryFrom> for Diff { }, Delta::Deleted => { let diff_file = delta.old_file(); - let path = diff_file.path().ok_or(error::Diff::PathUnavailable)?; - let path = Path::try_from(path.to_path_buf())?; - + let path = diff_file + .path() + .ok_or(error::Diff::PathUnavailable)? + .to_path_buf(); let patch = Patch::from_diff(&git_diff, idx)?; if let Some(patch) = patch { diff.add_deleted_file( @@ -139,9 +137,10 @@ impl<'a> TryFrom> for Diff { }, Delta::Modified => { let diff_file = delta.new_file(); - let path = diff_file.path().ok_or(error::Diff::PathUnavailable)?; - let path = Path::try_from(path.to_path_buf())?; - + let path = diff_file + .path() + .ok_or(error::Diff::PathUnavailable)? + .to_path_buf(); let patch = Patch::from_diff(&git_diff, idx)?; if let Some(patch) = patch { @@ -200,10 +199,7 @@ impl<'a> TryFrom> for Diff { .path() .ok_or(error::Diff::PathUnavailable)?; - let old_path = Path::try_from(old.to_path_buf())?; - let new_path = Path::try_from(new.to_path_buf())?; - - diff.add_moved_file(old_path, new_path); + diff.add_moved_file(old.to_path_buf(), new.to_path_buf()); }, Delta::Copied => { let old = delta @@ -215,10 +211,7 @@ impl<'a> TryFrom> for Diff { .path() .ok_or(error::Diff::PathUnavailable)?; - let old_path = Path::try_from(old.to_path_buf())?; - let new_path = Path::try_from(new.to_path_buf())?; - - diff.add_copied_file(old_path, new_path); + diff.add_copied_file(old.to_path_buf(), new.to_path_buf()); }, status => { return Err(error::Diff::DeltaUnhandled(status)); diff --git a/radicle-surf/src/file_system.rs b/radicle-surf/src/file_system.rs index 5943f94..b1c30ab 100644 --- a/radicle-surf/src/file_system.rs +++ b/radicle-surf/src/file_system.rs @@ -112,8 +112,3 @@ pub mod directory; pub use directory::{Directory, Entries, Entry, File, FileContent}; -mod error; -pub use error::Error; -mod path; - -pub use self::path::*; diff --git a/radicle-surf/src/file_system/directory.rs b/radicle-surf/src/file_system/directory.rs index 8fbf11f..21b3d1b 100644 --- a/radicle-surf/src/file_system/directory.rs +++ b/radicle-surf/src/file_system/directory.rs @@ -22,18 +22,15 @@ use std::{ collections::BTreeMap, - convert::{Infallible, Into}, - path, + convert::{Infallible, Into as _}, + path::{Path, PathBuf}, }; use git2::Blob; use radicle_git_ext::{is_not_found_err, Oid}; use radicle_std_ext::result::ResultExt as _; -use crate::{ - file_system::{path::*, Error}, - git::{Repository, Revision}, -}; +use crate::git::{Repository, Revision}; pub mod error { use thiserror::Error; @@ -52,8 +49,6 @@ pub mod error { pub enum Entry { #[error("the entry name was not valid UTF-8")] Utf8Error, - #[error(transparent)] - Label(#[from] super::Error), } #[derive(Debug, Error, PartialEq)] @@ -74,11 +69,32 @@ pub mod error { /// [`File::content`]. #[derive(Clone, PartialEq, Eq, Debug)] pub struct File { - pub(crate) name: Label, - pub(crate) id: Oid, + /// The name of the file. + name: String, + /// The relative path of the file, not including the `name`, + /// in respect to the root of the git repository. + prefix: PathBuf, + /// The object identifier of the git blob of this file. + id: Oid, } impl File { + /// Construct a new `File`. + /// + /// The `path` must be the prefix location of the directory, and + /// so should not end in `name`. + /// + /// The `id` must point to a git blob. + pub(crate) fn new(name: String, prefix: PathBuf, id: Oid) -> Self { + debug_assert!( + !prefix.ends_with(&name), + "prefix = {:?}, name = {}", + prefix, + name + ); + Self { name, prefix, id } + } + /// The name of this `File`. pub fn name(&self) -> &str { self.name.as_str() @@ -89,17 +105,18 @@ impl File { self.id } - /// Create a new `File` with the `name` and `id` provided. + /// Return the exact path for this `File`, including the `name` of + /// the directory itself. /// - /// The `id` must point to a `git` blob. - pub fn new(name: String, id: Oid) -> Self { - Self { - name: Label { - label: name, - hidden: false, - }, - id, - } + /// The path is relative to the git repository root. + pub fn path(&self) -> PathBuf { + self.prefix.join(&self.name) + } + + /// Return the [`Path`] where this `File` is located, relative to the + /// git repository root. + pub fn location(&self) -> &Path { + &self.prefix } /// Get the [`FileContent`] for this `File`. @@ -140,12 +157,12 @@ impl<'a> FileContent<'a> { /// A representations of a [`Directory`]'s entries. pub struct Entries { - listing: BTreeMap, + listing: BTreeMap, } impl Entries { - /// Return the [`Label`]s of each [`Entry`]. - pub fn names(&self) -> impl Iterator { + /// Return the name of each [`Entry`]. + pub fn names(&self) -> impl Iterator { self.listing.keys() } @@ -154,8 +171,8 @@ impl Entries { self.listing.values() } - /// Return each [`Label`] and [`Entry`]. - pub fn iter(&self) -> impl Iterator { + /// Return each [`Entry`] and its name. + pub fn iter(&self) -> impl Iterator { self.listing.iter() } } @@ -172,22 +189,22 @@ pub enum Entry { impl Entry { /// Get a label for the `Entriess`, either the name of the [`File`] /// or the name of the [`Directory`]. - pub fn label(&self) -> &Label { + pub fn name(&self) -> &String { match self { Entry::File(file) => &file.name, Entry::Directory(directory) => directory.name(), } } - pub(crate) fn from_entry(entry: &git2::TreeEntry) -> Result, error::Entry> { - let name = Label { - label: entry.name().ok_or(error::Entry::Utf8Error)?.to_string(), - hidden: false, - }; + pub(crate) fn from_entry( + entry: &git2::TreeEntry, + path: PathBuf, + ) -> Result, error::Entry> { + let name = entry.name().ok_or(error::Entry::Utf8Error)?.to_string(); let id = entry.id().into(); Ok(entry.kind().and_then(|kind| match kind { - git2::ObjectType::Tree => Some(Self::Directory(Directory { name, id })), - git2::ObjectType::Blob => Some(Self::File(File { name, id })), + git2::ObjectType::Tree => Some(Self::Directory(Directory::new(name, path, id))), + git2::ObjectType::Blob => Some(Self::File(File::new(name, path, id))), _ => None, })) } @@ -204,21 +221,63 @@ impl Entry { /// [git-tree]: https://git-scm.com/book/en/v2/Git-Internals-Git-Objects #[derive(Debug, Clone, PartialEq, Eq)] pub struct Directory { - pub(crate) name: Label, - pub(crate) id: Oid, + /// The name of the directoy. + name: String, + /// The relative path of the directory, not including the `name`, + /// in respect to the root of the git repository. + prefix: PathBuf, + /// The object identifier of the git tree of this directory. + id: Oid, } impl Directory { - /// Get the name of the current `Directory`. - pub fn name(&self) -> &Label { - &self.name + /// Creates a directory given its `id`. + /// + /// The `name` and `prefix` are both set to be empty. + /// + /// The `id` must point to a `git` tree. + pub(crate) fn root(id: Oid) -> Self { + Self::new("".to_string(), PathBuf::new(), id) } /// Creates a directory given its `name` and `id`. /// + /// The `path` must be the prefix location of the directory, and + /// so should not end in `name`. + /// /// The `id` must point to a `git` tree. - pub fn new(name: Label, id: Oid) -> Self { - Self { name, id } + pub(crate) fn new(name: String, prefix: PathBuf, id: Oid) -> Self { + debug_assert!( + name.is_empty() || !prefix.ends_with(&name), + "prefix = {:?}, name = {}", + prefix, + name + ); + Self { name, prefix, id } + } + + /// Get the name of the current `Directory`. + pub fn name(&self) -> &String { + &self.name + } + + /// The object identifier of this `File`. + pub fn id(&self) -> Oid { + self.id + } + + /// Return the exact path for this `Directory`, including the `name` of the + /// directory itself. + /// + /// The path is relative to the git repository root. + pub fn path(&self) -> PathBuf { + self.prefix.join(&self.name) + } + + /// Return the [`Path`] where this `Directory` is located, relative to the + /// git repository root. + pub fn location(&self) -> &Path { + &self.prefix } /// Return the [`Entries`] for this `Directory`'s `Oid`. @@ -238,15 +297,15 @@ impl Directory { let mut error = None; // Walks only the first level of entries. - tree.walk(git2::TreeWalkMode::PreOrder, |_s, entry| { - match Entry::from_entry(entry) { + tree.walk(git2::TreeWalkMode::PreOrder, |path, entry| { + match Entry::from_entry(entry, Path::new(path).to_path_buf()) { Ok(Some(entry)) => match entry { Entry::File(_) => { - entries.insert(entry.label().clone(), entry); + entries.insert(entry.name().clone(), entry); git2::TreeWalkResult::Ok }, Entry::Directory(_) => { - entries.insert(entry.label().clone(), entry); + entries.insert(entry.name().clone(), entry); // Skip nested directories git2::TreeWalkResult::Skip }, @@ -266,45 +325,52 @@ impl Directory { } /// Find the [`Entry`] found at `path`, if it exists. - pub fn find_entry( + pub fn find_entry

( &self, - path: &path::Path, + path: &P, repo: &Repository, - ) -> Result, crate::git::Error> { + ) -> Result, crate::git::Error> + where + P: AsRef, + { // Search the path in git2 tree. let git2_tree = repo.git2_repo().find_tree(self.id.into())?; let entry = git2_tree - .get_path(path) + .get_path(path.as_ref()) .map(Some) .or_matches::(is_not_found_err, || Ok(None))?; Ok(entry - .and_then(|entry| Entry::from_entry(&entry).transpose()) + .and_then(|entry| Entry::from_entry(&entry, path.as_ref().to_path_buf()).transpose()) .transpose() .unwrap()) } /// Find the `Oid`, for a [`File`], found at `path`, if it exists. - pub fn find_file( + pub fn find_file

( &self, - path: Path, + path: &P, repo: &Repository, - ) -> Result, crate::git::Error> { - let path_buf: std::path::PathBuf = (&path).into(); - Ok(match self.find_entry(path_buf.as_path(), repo)? { + ) -> Result, crate::git::Error> + where + P: AsRef, + { + Ok(match self.find_entry(path, repo)? { Some(Entry::File(f)) => Some(f.id), _ => None, }) } /// Find the `Directory` found at `path`, if it exists. - pub fn find_directory( + pub fn find_directory

( &self, - path: Path, + path: P, repo: &Repository, - ) -> Result, crate::git::Error> { - let path_buf: std::path::PathBuf = (&path).into(); - Ok(match self.find_entry(path_buf.as_path(), repo)? { + ) -> Result, crate::git::Error> + where + P: AsRef, + { + Ok(match self.find_entry(&path, repo)? { Some(Entry::Directory(d)) => Some(d), _ => None, }) @@ -313,7 +379,7 @@ impl Directory { // TODO(fintan): This is going to be a bit trickier so going to leave it out for // now #[allow(dead_code)] - fn fuzzy_find(_label: Label) -> Vec { + fn fuzzy_find(_label: &Path) -> Vec { unimplemented!() } diff --git a/radicle-surf/src/file_system/error.rs b/radicle-surf/src/file_system/error.rs deleted file mode 100644 index 20cc2a3..0000000 --- a/radicle-surf/src/file_system/error.rs +++ /dev/null @@ -1,79 +0,0 @@ -// This file is part of radicle-surf -// -// -// Copyright (C) 2019-2020 The Radicle Team -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License version 3 or -// later as published by the Free Software Foundation. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -//! Errors that can occur within the file system logic. -//! -//! These errors occur due to [`Label`](super::path::Label) and -//! [`Path`](super::path::Path) parsing when using their respective `TryFrom` -//! instances. - -use std::ffi::OsStr; -use thiserror::Error; - -pub(crate) const EMPTY_PATH: Error = Error::Path(PathError::Empty); -pub(crate) const EMPTY_LABEL: Error = Error::Label(LabelError::Empty); - -/// Build an [`Error::Label(LabelError::InvalidUTF8)`] from an -/// [`OsStr`](std::ffi::OsStr) -pub(crate) fn label_invalid_utf8(item: &OsStr) -> Error { - Error::Label(LabelError::InvalidUTF8 { - label: item.to_string_lossy().into(), - }) -} - -/// Build an [`Error::Label(LabelError::ContainsSlash)`] from a [`str`] -pub(crate) fn label_has_slash(item: &str) -> Error { - Error::Label(LabelError::ContainsSlash { label: item.into() }) -} - -/// Error type for all file system errors that can occur. -#[derive(Debug, Clone, PartialEq, Eq, Error)] -#[non_exhaustive] -pub enum Error { - /// A `LabelError` specific error for parsing a - /// [`Path`](super::path::Label). - #[error(transparent)] - Label(#[from] LabelError), - /// A `PathError` specific error for parsing a [`Path`](super::path::Path). - #[error(transparent)] - Path(#[from] PathError), -} - -/// Parse errors for when parsing a string to a [`Path`](super::path::Path). -#[derive(Debug, Clone, PartialEq, Eq, Error)] -#[non_exhaustive] -pub enum PathError { - /// An error signifying that a [`Path`](super::path::Path) is empty. - #[error("path is empty")] - Empty, -} - -/// Parse errors for when parsing a string to a [`Label`](super::path::Label). -#[derive(Debug, Clone, PartialEq, Eq, Error)] -#[non_exhaustive] -pub enum LabelError { - /// An error signifying that a [`Label`](super::path::Label) is contains - /// invalid UTF-8. - #[error("label '{label}' contains invalid UTF-8")] - InvalidUTF8 { label: String }, - /// An error signifying that a [`Label`](super::path::Label) contains a `/`. - #[error("label '{label}' contains a slash")] - ContainsSlash { label: String }, - /// An error signifying that a [`Label`](super::path::Label) is empty. - #[error("label is empty")] - Empty, -} diff --git a/radicle-surf/src/file_system/path.rs b/radicle-surf/src/file_system/path.rs deleted file mode 100644 index 332ed23..0000000 --- a/radicle-surf/src/file_system/path.rs +++ /dev/null @@ -1,453 +0,0 @@ -// This file is part of radicle-surf -// -// -// Copyright (C) 2019-2020 The Radicle Team -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License version 3 or -// later as published by the Free Software Foundation. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -use std::fmt::Write as _; - -use crate::{file_system::error, nonempty::split_last}; -use nonempty::NonEmpty; -use std::{convert::TryFrom, ffi::CString, fmt, ops::Deref, path, str::FromStr}; - -#[cfg(feature = "serialize")] -use serde::{Serialize, Serializer}; - -pub mod unsound; - -/// `Label` is a special case of a `String` identifier for -/// [`Directory`](`crate::file_system::directory::Directory`) and -/// [`File`](`crate::file_system::directory::File`) names, and is used in -/// [`Path`] as the component parts of a path. -/// -/// A `Label` should not be empty or contain `/`s. It is encouraged to use the -/// `TryFrom` instance to create a `Label`. -#[cfg_attr(feature = "serialize", derive(Serialize))] -#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct Label { - pub(crate) label: String, - pub(crate) hidden: bool, -} - -impl Deref for Label { - type Target = String; - - fn deref(&self) -> &Self::Target { - &self.label - } -} - -/// The label for the root directory. -pub const ROOT_LABEL: &str = "~"; - -impl Label { - /// The root label for the root directory, i.e. `"~"`. - /// - /// Prefer creating a root [`Path`], by using - /// [`Path::root`](struct.Path.html#method.root). - /// - /// # Examples - /// - /// ``` - /// use radicle_surf::file_system::{Label, Path}; - /// - /// let root = Path::root(); - /// assert_eq!(*root.split_first().0, Label::root()); - /// ``` - pub fn root() -> Self { - Label { - label: ROOT_LABEL.into(), - hidden: false, - } - } - - /// Check that the label is equivalent to [`Label::root`]. - /// - /// # Examples - /// - /// ``` - /// use radicle_surf::file_system::Label; - /// use radicle_surf::file_system::unsound; - /// - /// let root = unsound::label::new("~"); - /// assert!(root.is_root()); - /// ``` - pub fn is_root(&self) -> bool { - *self == Self::root() - } -} - -impl fmt::Display for Label { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.label) - } -} - -impl TryFrom<&str> for Label { - type Error = error::Error; - - fn try_from(item: &str) -> Result { - if item.is_empty() { - Err(error::EMPTY_LABEL) - } else if item.contains('/') { - Err(error::label_has_slash(item)) - } else { - Ok(Label { - label: item.into(), - hidden: false, - }) - } - } -} - -impl FromStr for Label { - type Err = error::Error; - - fn from_str(item: &str) -> Result { - Label::try_from(item) - } -} - -/// A non-empty set of [`Label`]s to define a path to a directory or file. -/// -/// `Path` tends to be used for insertion or find operations. -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct Path(pub NonEmpty

(mut self, path: P) -> Self + where + P: AsRef, + { + self.filter_by = Some(FilterBy::File { + path: path.as_ref().to_path_buf(), + }); self } } diff --git a/radicle-surf/src/git/repo.rs b/radicle-surf/src/git/repo.rs index faa97c7..ad2cb30 100644 --- a/radicle-surf/src/git/repo.rs +++ b/radicle-surf/src/git/repo.rs @@ -15,7 +15,12 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see . -use std::{collections::BTreeSet, convert::TryFrom, path::PathBuf, str}; +use std::{ + collections::BTreeSet, + convert::TryFrom, + path::{Path, PathBuf}, + str, +}; use git_ref_format::{refspec::QualifiedPattern, Qualified}; use radicle_git_ext::Oid; @@ -23,8 +28,7 @@ use thiserror::Error; use crate::{ diff::{self, *}, - file_system, - file_system::{directory::FileContent, Directory, Label}, + file_system::{directory::FileContent, Directory}, git::{ commit, glob, @@ -58,9 +62,6 @@ pub enum Error { /// An error that comes from performing a *diff* operations. #[error(transparent)] Diff(#[from] diff::git::error::Diff), - /// An error that comes from performing a [`crate::file_system`] operation. - #[error(transparent)] - FileSystem(#[from] file_system::Error), /// A wrapper around the generic [`git2::Error`]. #[error(transparent)] Git(#[from] git2::Error), @@ -72,7 +73,7 @@ pub enum Error { NotQualified(String), /// The requested file was not found. #[error("path not found for: {0}")] - PathNotFound(file_system::Path), + PathNotFound(PathBuf), #[error(transparent)] RefFormat(#[from] git_ref_format::Error), #[error(transparent)] @@ -187,19 +188,16 @@ impl Repository { .map_err(|err| Error::ToCommit(err.into()))?; let git2_commit = self.inner.find_commit((commit.id).into())?; let tree = git2_commit.as_object().peel_to_tree()?; - Ok(Directory { - name: Label::root(), - id: tree.id().into(), - }) + Ok(Directory::root(tree.id().into())) } /// Returns the last commit, if exists, for a `path` in the history of /// `rev`. - pub fn last_commit( - &self, - path: file_system::Path, - rev: C, - ) -> Result, Error> { + pub fn last_commit(&self, path: P, rev: C) -> Result, Error> + where + P: AsRef, + C: ToCommit, + { let history = self.history(rev)?; history.by_path(path).next().transpose() } @@ -239,17 +237,20 @@ impl Repository { } /// Retrieves the file with `path` in this commit. - pub fn get_commit_file( - &self, - rev: &R, - path: file_system::Path, - ) -> Result { + pub fn get_commit_file(&self, rev: &R, path: &P) -> Result + where + P: AsRef, + R: Revision, + { + let path = path.as_ref(); let id = self.object_id(rev)?; let commit = self.get_git2_commit(id)?; let tree = commit.tree()?; - let entry = tree.get_path(PathBuf::from(&path).as_ref())?; + let entry = tree.get_path(path)?; let object = entry.to_object(self.git2_repo())?; - let blob = object.into_blob().map_err(|_| Error::PathNotFound(path))?; + let blob = object + .into_blob() + .map_err(|_| Error::PathNotFound(path.to_path_buf()))?; Ok(FileContent::new(blob)) } @@ -355,16 +356,19 @@ impl Repository { Ok(other == git2_oid || is_descendant) } - pub(crate) fn diff_commit_and_parents( + pub(crate) fn diff_commit_and_parents

( &self, - path: &file_system::Path, + path: &P, commit: &git2::Commit, - ) -> Result, Error> { + ) -> Result, Error> + where + P: AsRef, + { let mut parents = commit.parents(); - let diff = self.diff_commits(Some(path), parents.next().as_ref(), commit)?; + let diff = self.diff_commits(Some(path.as_ref()), parents.next().as_ref(), commit)?; if let Some(_delta) = diff.deltas().next() { - Ok(Some(path.clone())) + Ok(Some(path.as_ref().to_path_buf())) } else { Ok(None) } @@ -372,7 +376,7 @@ impl Repository { fn diff_commits( &self, - path: Option<&file_system::Path>, + path: Option<&Path>, from: Option<&git2::Commit>, to: &git2::Commit, ) -> Result { diff --git a/radicle-surf/src/lib.rs b/radicle-surf/src/lib.rs index 63d8340..815fd75 100644 --- a/radicle-surf/src/lib.rs +++ b/radicle-surf/src/lib.rs @@ -99,6 +99,3 @@ pub use revision::Revision; pub mod syntax; #[cfg(feature = "syntax")] pub use syntax::SYNTAX_SET; - -// Private modules -mod nonempty; diff --git a/radicle-surf/src/nonempty.rs b/radicle-surf/src/nonempty.rs deleted file mode 100644 index 37110f5..0000000 --- a/radicle-surf/src/nonempty.rs +++ /dev/null @@ -1,33 +0,0 @@ -// This file is part of radicle-surf -// -// -// Copyright (C) 2019-2020 The Radicle Team -// -// This program is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License version 3 or -// later as published by the Free Software Foundation. -// -// This program is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with this program. If not, see . - -use nonempty::NonEmpty; - -pub fn split_last(non_empty: NonEmpty) -> (Vec, T) -where - T: Eq, -{ - let (head, mut tail) = non_empty.into(); - let last = tail.pop(); - match last { - None => (vec![], head), - Some(last) => { - tail.insert(0, head); - (tail, last) - }, - } -} diff --git a/radicle-surf/src/object.rs b/radicle-surf/src/object.rs index 10f1d07..4ef08b3 100644 --- a/radicle-surf/src/object.rs +++ b/radicle-surf/src/object.rs @@ -18,6 +18,8 @@ //! Common definitions for git objects (blob and tree). //! See git [doc](https://git-scm.com/book/en/v2/Git-Internals-Git-Objects) for more details. +use std::path::PathBuf; + #[cfg(feature = "serialize")] use serde::{ ser::{SerializeStruct as _, Serializer}, @@ -30,11 +32,7 @@ pub use blob::{Blob, BlobContent}; pub mod tree; pub use tree::{tree, Tree, TreeEntry}; -use crate::{ - commit, - file_system::{self, directory}, - git, -}; +use crate::{commit, file_system::directory, git}; /// Git object types. /// @@ -91,15 +89,11 @@ pub enum Error { #[error(transparent)] Directory(#[from] directory::error::Directory), - /// An error occurred during a file system operation. - #[error(transparent)] - FileSystem(#[from] file_system::Error), - /// An error occurred during a git operation. #[error(transparent)] Git(#[from] git::Error), /// Trying to find a file path which could not be found. #[error("the path '{0}' was not found")] - PathNotFound(file_system::Path), + PathNotFound(PathBuf), } diff --git a/radicle-surf/src/object/blob.rs b/radicle-surf/src/object/blob.rs index ade6a0b..b8bef4d 100644 --- a/radicle-surf/src/object/blob.rs +++ b/radicle-surf/src/object/blob.rs @@ -18,7 +18,10 @@ //! Represents git object type 'blob', i.e. actual file contents. //! See git [doc](https://git-scm.com/book/en/v2/Git-Internals-Git-Objects) for more details. -use std::str::{self, FromStr as _}; +use std::{ + path::{Path, PathBuf}, + str, +}; #[cfg(feature = "serialize")] use serde::{ @@ -28,7 +31,6 @@ use serde::{ use crate::{ commit, - file_system, git::Repository, object::{Error, Info, ObjectType}, revision::Revision, @@ -44,7 +46,7 @@ pub struct Blob { /// Extra info for the file. pub info: Info, /// Absolute path to the object from the root of the repo. - pub path: String, + pub path: PathBuf, } impl Blob { @@ -114,38 +116,42 @@ impl Serialize for BlobContent { /// /// Will return [`Error`] if the project doesn't exist or a surf interaction /// fails. -pub fn blob( - repo: &Repository, - maybe_revision: Option, - path: &str, -) -> Result { +pub fn blob

(repo: &Repository, maybe_revision: Option, path: &P) -> Result +where + P: AsRef, +{ make_blob(repo, maybe_revision, path, content) } -fn make_blob( +fn make_blob( repo: &Repository, maybe_revision: Option, - path: &str, + path: &P, content: C, ) -> Result where + P: AsRef, C: FnOnce(&[u8]) -> BlobContent, { + let path = path.as_ref(); let revision = maybe_revision.unwrap(); let root = repo.root_dir(&revision)?; - let p = file_system::Path::from_str(path)?; let file = root - .find_file(p.clone(), repo)? - .ok_or_else(|| Error::PathNotFound(p.clone()))?; - - let mut commit_path = file_system::Path::root(); - commit_path.append(p.clone()); + .find_file(&path, repo)? + .ok_or_else(|| Error::PathNotFound(path.to_path_buf()))?; let last_commit = repo - .last_commit(commit_path, &revision)? + .last_commit(path, &revision)? .map(|c| commit::Header::from(&c)); - let (_rest, last) = p.split_last(); + // TODO: fuck this + let name = path + .file_name() + .unwrap() + .to_os_string() + .into_string() + .ok() + .unwrap(); let file_content = repo.file_content(file)?; let content = content(file_content.as_bytes()); @@ -153,11 +159,11 @@ where Ok(Blob { content, info: Info { - name: last.to_string(), + name, object_type: ObjectType::Blob, last_commit, }, - path: path.to_string(), + path: path.to_path_buf(), }) } diff --git a/radicle-surf/src/object/tree.rs b/radicle-surf/src/object/tree.rs index 88c7bfd..e60c26c 100644 --- a/radicle-surf/src/object/tree.rs +++ b/radicle-surf/src/object/tree.rs @@ -18,7 +18,7 @@ //! Represents git object type 'tree', i.e. like directory entries in Unix. //! See git [doc](https://git-scm.com/book/en/v2/Git-Internals-Git-Objects) for more details. -use std::str::FromStr as _; +use std::path::{Path, PathBuf}; use git_ref_format::refname; #[cfg(feature = "serialize")] @@ -29,7 +29,7 @@ use serde::{ use crate::{ commit, - file_system::{self, directory}, + file_system::directory, git::Repository, object::{Error, Info, ObjectType}, revision::Revision, @@ -38,7 +38,7 @@ use crate::{ /// Result of a directory listing, carries other trees and blobs. pub struct Tree { /// Absolute path to the tree object from the repo root. - pub path: String, + pub path: PathBuf, /// Entries listed in that tree result. pub entries: Vec, /// Extra info for the tree object. @@ -65,7 +65,7 @@ pub struct TreeEntry { /// Extra info for the entry. pub info: Info, /// Absolute path to the object from the root of the repo. - pub path: String, + pub path: PathBuf, } #[cfg(feature = "serialize")] @@ -86,12 +86,15 @@ impl Serialize for TreeEntry { /// # Errors /// /// Will return [`Error`] if any of the surf interactions fail. -pub fn tree( +pub fn tree

( repo: &Repository, maybe_revision: Option, - maybe_prefix: Option, -) -> Result { - let prefix = maybe_prefix.unwrap_or_default(); + maybe_prefix: Option<&P>, +) -> Result +where + P: AsRef, +{ + let maybe_prefix = maybe_prefix.map(|p| p.as_ref()); let rev = match maybe_revision { Some(r) => r, None => Revision::Branch { @@ -100,51 +103,42 @@ pub fn tree( }, }; - let path = if prefix == "/" || prefix.is_empty() { - file_system::Path::root() - } else { - file_system::Path::from_str(&prefix)? + let prefix_dir = match maybe_prefix { + None => repo.root_dir(&rev)?, + Some(path) => repo + .root_dir(&rev)? + .find_directory(path, repo)? + .ok_or_else(|| Error::PathNotFound(path.to_path_buf()))?, }; - let root_dir = repo.root_dir(&rev)?; - let prefix_dir = if path.is_root() { - root_dir - } else { - root_dir - .find_directory(path.clone(), repo)? - .ok_or_else(|| Error::PathNotFound(path.clone()))? - }; + let mut entries = prefix_dir + .entries(repo)? + .entries() + .fold(Vec::new(), |mut entries, entry| { + let entry_path = match maybe_prefix { + Some(path) => { + let mut path = path.to_path_buf(); + path.push(entry.name()); + path + }, + None => PathBuf::new(), + }; - let mut entries = - prefix_dir - .entries(repo)? - .iter() - .fold(Vec::new(), |mut entries, (label, entry)| { - let entry_path = if path.is_root() { - file_system::Path::new(label.clone()) - } else { - let mut p = path.clone(); - p.push(label.clone()); - p - }; - let mut commit_path = file_system::Path::root(); - commit_path.append(entry_path.clone()); + let info = Info { + name: entry.name().clone(), + object_type: match entry { + directory::Entry::Directory(_) => ObjectType::Tree, + directory::Entry::File { .. } => ObjectType::Blob, + }, + last_commit: None, + }; - let info = Info { - name: label.to_string(), - object_type: match entry { - directory::Entry::Directory(_) => ObjectType::Tree, - directory::Entry::File { .. } => ObjectType::Blob, - }, - last_commit: None, - }; - - entries.push(TreeEntry { - info, - path: entry_path.to_string(), - }); - entries + entries.push(TreeEntry { + info, + path: entry_path, }); + entries + }); // We want to ensure that in the response Tree entries come first. `Ord` being // derived on the enum ensures Variant declaration order. @@ -152,17 +146,15 @@ pub fn tree( // https://doc.rust-lang.org/std/cmp/trait.Ord.html#derivable entries.sort_by(|a, b| a.info.object_type.cmp(&b.info.object_type)); - let last_commit = if path.is_root() { + let last_commit = if maybe_prefix.is_none() { let history = repo.history(&rev)?; Some(commit::Header::from(history.head())) } else { None }; - let name = if path.is_root() { - "".into() - } else { - let (_first, last) = path.split_last(); - last.to_string() + let name = match maybe_prefix { + None => "".into(), + Some(path) => path.file_name().unwrap().to_str().unwrap().to_string(), }; let info = Info { name, @@ -171,7 +163,7 @@ pub fn tree( }; Ok(Tree { - path: prefix, + path: maybe_prefix.map_or(PathBuf::new(), |path| path.to_path_buf()), entries, info, }) diff --git a/radicle-surf/t/src/file_system.rs b/radicle-surf/t/src/file_system.rs index cc4486b..7bf97ef 100644 --- a/radicle-surf/t/src/file_system.rs +++ b/radicle-surf/t/src/file_system.rs @@ -3,31 +3,6 @@ //! Unit tests for radicle_surf::file_system -#[cfg(test)] -mod path { - use radicle_surf::file_system::unsound; - - #[test] - fn split_last_root_and_foo() { - let path = unsound::path::new("foo"); - assert_eq!(path.split_last(), (vec![], unsound::label::new("foo"))); - } - - #[test] - fn split_last_same_labels() { - // An interesting case for when first == last, but doesn't imply a singleton - // Path. - let path = unsound::path::new("foo/bar/foo"); - assert_eq!( - path.split_last(), - ( - vec![unsound::label::new("foo"), unsound::label::new("bar")], - unsound::label::new("foo") - ) - ); - } -} - #[cfg(test)] mod directory { use git_ref_format::refname; diff --git a/radicle-surf/t/src/git.rs b/radicle-surf/t/src/git.rs index c16d493..534aea0 100644 --- a/radicle-surf/t/src/git.rs +++ b/radicle-surf/t/src/git.rs @@ -7,7 +7,7 @@ use radicle_surf::git::{Author, Commit}; use radicle_surf::{ diff::*, - file_system::{directory, unsound, Path}, + file_system::directory, git::{Branch, Error, Glob, Oid, Repository}, }; @@ -246,7 +246,10 @@ mod last_commit { use git_ref_format::refname; use super::*; - use std::str::FromStr; + use std::{ + path::{Path, PathBuf}, + str::FromStr, + }; #[test] fn readme_missing_and_memory() { @@ -257,10 +260,7 @@ mod last_commit { // memory.rs is commited later so it should not exist here. let memory_last_commit_oid = repo - .last_commit( - Path::with_root(&[unsound::label::new("src"), unsound::label::new("memory.rs")]), - oid, - ) + .last_commit(Path::new("src/memory.rs"), oid) .expect("Failed to get last commit") .map(|commit| commit.id); @@ -268,7 +268,7 @@ mod last_commit { // README.md exists in this commit. let readme_last_commit = repo - .last_commit(Path::with_root(&[unsound::label::new("README.md")]), oid) + .last_commit(Path::new("README.md"), oid) .expect("Failed to get last commit") .map(|commit| commit.id); @@ -286,7 +286,7 @@ mod last_commit { let expected_commit_id = Oid::from_str("f3a089488f4cfd1a240a9c01b3fcc4c34a4e97b2").unwrap(); let folder_svelte = repo - .last_commit(unsound::path::new("~/examples/Folder.svelte"), oid) + .last_commit(Path::new("examples/Folder.svelte"), oid) .expect("Failed to get last commit") .map(|commit| commit.id); @@ -305,7 +305,7 @@ mod last_commit { let nested_directory_tree_commit_id = repo .last_commit( - unsound::path::new("~/this/is/a/really/deeply/nested/directory/tree"), + Path::new("this/is/a/really/deeply/nested/directory/tree"), oid, ) .expect("Failed to get last commit") @@ -327,13 +327,13 @@ mod last_commit { let expected_commit_id = Oid::from_str("a0dd9122d33dff2a35f564d564db127152c88e02").unwrap(); let backslash_commit_id = repo - .last_commit(unsound::path::new("~/special/faux\\path"), oid) + .last_commit(Path::new(r"special/faux\\path"), oid) .expect("Failed to get last commit") .map(|commit| commit.id); assert_eq!(backslash_commit_id, Some(expected_commit_id)); let ogre_commit_id = repo - .last_commit(unsound::path::new("~/special/👹👹👹"), oid) + .last_commit(Path::new("special/👹👹👹"), oid) .expect("Failed to get last commit") .map(|commit| commit.id); assert_eq!(ogre_commit_id, Some(expected_commit_id)); @@ -345,7 +345,7 @@ mod last_commit { .expect("Could not retrieve ./data/git-platinum as git repository"); let rev = Branch::local(refname!("master")); let root_last_commit_id = repo - .last_commit(Path::root(), &rev) + .last_commit(&PathBuf::new(), &rev) .expect("Failed to get last commit") .map(|commit| commit.id); @@ -362,7 +362,7 @@ mod last_commit { let repo = Repository::open(GIT_PLATINUM) .expect("Could not retrieve ./data/git-platinum as git repository"); let history = repo.history(&Branch::local(refname!("dev"))).unwrap(); - let file_commit = history.by_path(unsound::path::new("~/bin/cat")).next(); + let file_commit = history.by_path(Path::new("bin/cat")).next(); assert!(file_commit.is_some()); println!("file commit: {:?}", &file_commit); } @@ -373,7 +373,7 @@ mod diff { use super::*; use git_ref_format::refname; use pretty_assertions::assert_eq; - use std::str::FromStr; + use std::{path::Path, str::FromStr}; #[test] fn test_initial_diff() -> Result<(), Error> { @@ -386,7 +386,7 @@ mod diff { let expected_diff = Diff { created: vec![CreateFile { - path: Path::with_root(&[unsound::label::new("README.md")]), + path: Path::new("README.md").to_path_buf(), diff: FileDiff::Plain { hunks: vec![Hunk { header: Line::from(b"@@ -0,0 +1 @@\n".to_vec()), @@ -434,7 +434,7 @@ mod diff { moved: vec![], copied: vec![], modified: vec![ModifiedFile { - path: Path::with_root(&[unsound::label::new("README.md")]), + path: Path::new("README.md").to_path_buf(), diff: FileDiff::Plain { hunks: vec![Hunk { header: Line::from(b"@@ -1 +1,2 @@\n".to_vec()), @@ -474,16 +474,16 @@ mod diff { assert_eq!(diff.moved.len(), 1); assert_eq!(diff.modified.len(), 2); for c in diff.created.iter() { - println!("created: {}", &c.path); + println!("created: {:?}", &c.path); } for d in diff.deleted.iter() { - println!("deleted: {}", &d.path); + println!("deleted: {:?}", &d.path); } for m in diff.moved.iter() { - println!("moved: {} -> {}", &m.old_path, &m.new_path); + println!("moved: {:?} -> {:?}", &m.old_path, &m.new_path); } for m in diff.modified.iter() { - println!("modified: {}", &m.path); + println!("modified: {:?}", &m.path); } Ok(()) } @@ -717,6 +717,8 @@ mod reference { } mod code_browsing { + use std::path::Path; + use super::*; use git_ref_format::refname; @@ -740,7 +742,7 @@ mod code_browsing { repo, (0, 0), &mut |(count, indent_level), entry| { - println!("> {}{}", " ".repeat(indent_level * 4), entry.label()); + println!("> {}{}", " ".repeat(indent_level * 4), entry.name()); match entry { directory::Entry::File(_) => Ok((count + 1, indent_level)), directory::Entry::Directory(_) => Ok((count + 1, indent_level + 1)), @@ -775,11 +777,11 @@ mod code_browsing { fn test_file_history() { let repo = Repository::open(GIT_PLATINUM).unwrap(); let history = repo.history(&Branch::local(refname!("dev"))).unwrap(); - let path = unsound::path::new("README.md"); + let path = Path::new("README.md"); let mut file_history = history.by_path(path); let commit = file_history.next().unwrap().unwrap(); let file = repo - .get_commit_file(&commit.id, unsound::path::new("README.md")) + .get_commit_file(&commit.id, &Path::new("README.md")) .unwrap(); assert_eq!(file.size(), 67); } diff --git a/radicle-surf/t/src/lib.rs b/radicle-surf/t/src/lib.rs index 40d1e3e..b1277eb 100644 --- a/radicle-surf/t/src/lib.rs +++ b/radicle-surf/t/src/lib.rs @@ -6,6 +6,3 @@ mod git; #[cfg(test)] mod diff; - -#[cfg(test)] -mod file_system;