From 619a17a9c6695dee92852aaa6d410f102a2af04f Mon Sep 17 00:00:00 2001 From: Cyborus Date: Fri, 31 May 2024 11:56:08 -0400 Subject: [PATCH] improve specifying repo in issue and pr commands --- src/issues.rs | 125 +++++++++++++++++++++++++++++++++++-------- src/prs.rs | 145 ++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 209 insertions(+), 61 deletions(-) diff --git a/src/issues.rs b/src/issues.rs index ed2b9ab..cf1abd7 100644 --- a/src/issues.rs +++ b/src/issues.rs @@ -1,3 +1,5 @@ +use std::str::FromStr; + use clap::{Args, Subcommand}; use eyre::{eyre, OptionExt}; use forgejo_api::structs::{ @@ -11,8 +13,6 @@ use crate::repo::{RepoInfo, RepoName}; pub struct IssueCommand { #[clap(long, short = 'R')] remote: Option, - #[clap(long, short)] - repo: Option, #[clap(subcommand)] command: IssueSubcommand, } @@ -23,22 +23,26 @@ pub enum IssueSubcommand { title: String, #[clap(long)] body: Option, + #[clap(long, short)] + repo: Option, }, Edit { - issue: u64, + issue: IssueId, #[clap(subcommand)] command: EditCommand, }, Comment { - issue: u64, + issue: IssueId, body: Option, }, Close { - issue: u64, + issue: IssueId, #[clap(long, short)] with_msg: Option>, }, Search { + #[clap(long, short)] + repo: Option, query: Option, #[clap(long, short)] labels: Option, @@ -50,15 +54,36 @@ pub enum IssueSubcommand { state: Option, }, View { - id: u64, + id: IssueId, #[clap(subcommand)] command: Option, }, Browse { - id: Option, + id: Option, }, } +#[derive(Clone, Debug)] +pub struct IssueId { + pub repo: Option, + pub number: u64, +} + +impl FromStr for IssueId { + type Err = std::num::ParseIntError; + + fn from_str(s: &str) -> Result { + let (repo, number) = match s.rsplit_once("#") { + Some((repo, number)) => (Some(repo.to_owned()), number), + None => (None, s), + }; + Ok(Self { + repo, + number: number.parse()?, + }) + } +} + #[derive(clap::ValueEnum, Clone, Copy, Debug)] pub enum State { Open, @@ -98,19 +123,22 @@ pub enum ViewCommand { impl IssueCommand { pub async fn run(self, keys: &crate::KeyInfo, host_name: Option<&str>) -> eyre::Result<()> { use IssueSubcommand::*; - let repo = RepoInfo::get_current(host_name, self.repo.as_deref(), self.remote.as_deref())?; + let repo = RepoInfo::get_current(host_name, self.repo(), self.remote.as_deref())?; let api = keys.get_api(repo.host_url())?; - let repo = repo - .name() - .ok_or_eyre("couldn't get repo name, try specifying with --repo")?; + let repo = repo.name().ok_or_else(|| self.no_repo_error())?; match self.command { - Create { title, body } => create_issue(&repo, &api, title, body).await?, + Create { + repo: _, + title, + body, + } => create_issue(&repo, &api, title, body).await?, View { id, command } => match command.unwrap_or(ViewCommand::Body) { - ViewCommand::Body => view_issue(&repo, &api, id).await?, - ViewCommand::Comment { idx } => view_comment(&repo, &api, id, idx).await?, - ViewCommand::Comments => view_comments(&repo, &api, id).await?, + ViewCommand::Body => view_issue(&repo, &api, id.number).await?, + ViewCommand::Comment { idx } => view_comment(&repo, &api, id.number, idx).await?, + ViewCommand::Comments => view_comments(&repo, &api, id.number).await?, }, Search { + repo: _, query, labels, creator, @@ -119,19 +147,74 @@ impl IssueCommand { } => view_issues(&repo, &api, query, labels, creator, assignee, state).await?, Edit { issue, command } => match command { EditCommand::Title { new_title } => { - edit_title(&repo, &api, issue, new_title).await? + edit_title(&repo, &api, issue.number, new_title).await? + } + EditCommand::Body { new_body } => { + edit_body(&repo, &api, issue.number, new_body).await? } - EditCommand::Body { new_body } => edit_body(&repo, &api, issue, new_body).await?, EditCommand::Comment { idx, new_body } => { - edit_comment(&repo, &api, issue, idx, new_body).await? + edit_comment(&repo, &api, issue.number, idx, new_body).await? } }, - Close { issue, with_msg } => close_issue(&repo, &api, issue, with_msg).await?, - Browse { id } => browse_issue(&repo, &api, id).await?, - Comment { issue, body } => add_comment(&repo, &api, issue, body).await?, + Close { issue, with_msg } => close_issue(&repo, &api, issue.number, with_msg).await?, + Browse { id } => { + let number = id.as_ref().and_then(|s| { + let num_s = s.rsplit_once("#").map(|(_, b)| b).unwrap_or(s); + num_s.parse::().ok() + }); + browse_issue(&repo, &api, number).await? + } + Comment { issue, body } => add_comment(&repo, &api, issue.number, body).await?, } Ok(()) } + + fn repo(&self) -> Option<&str> { + use IssueSubcommand::*; + match &self.command { + Create { repo, .. } | Search { repo, .. } => repo.as_deref(), + View { id: issue, .. } + | Edit { issue, .. } + | Close { issue, .. } + | Comment { issue, .. } => issue.repo.as_deref(), + Browse { id, .. } => id.as_ref().and_then(|s| { + let repo = s.rsplit_once("#").map(|(a, _)| a).unwrap_or(s); + // Don't treat a lone issue number as a repo name + if repo.parse::().is_ok() { + None + } else { + Some(repo) + } + }), + } + } + + fn no_repo_error(&self) -> eyre::Error { + use IssueSubcommand::*; + match &self.command { + Create { repo, .. } | Search { repo, .. } => { + eyre::eyre!("can't figure what repo to access, try specifying with `--repo`") + } + View { id: issue, .. } + | Edit { issue, .. } + | Close { issue, .. } + | Comment { issue, .. } => eyre::eyre!( + "can't figure out what repo to access, try specifying with `{{owner}}/{{repo}}#{}`", + issue.number + ), + Browse { id, .. } => { + let number = id.as_ref().and_then(|s| { + let num_s = s.rsplit_once("#").map(|(_, b)| b).unwrap_or(s); + num_s.parse::().ok() + }); + if let Some(number) = number { + eyre::eyre!("can't figure out what repo to access, try specifying with `{{owner}}/{{repo}}#{}`", number) + } else { + eyre::eyre!("can't figure out what repo to access, try specifying with `{{owner}}/{{repo}}`") + } + } + } + } } async fn create_issue( diff --git a/src/prs.rs b/src/prs.rs index b1d7074..f9b2774 100644 --- a/src/prs.rs +++ b/src/prs.rs @@ -11,6 +11,7 @@ use forgejo_api::{ }; use crate::{ + issues::IssueId, repo::{RepoInfo, RepoName}, SpecialRender, }; @@ -20,9 +21,6 @@ pub struct PrCommand { /// The git remote to operate on. #[clap(long, short = 'R')] remote: Option, - /// The name of the remote repository to operate on. - #[clap(long, short)] - repo: Option, #[clap(subcommand)] command: PrSubcommand, } @@ -40,6 +38,9 @@ pub enum PrSubcommand { assignee: Option, #[clap(long, short)] state: Option, + /// The repo to search in + #[clap(long, short)] + repo: Option, }, /// Create a new pull request Create { @@ -56,11 +57,14 @@ pub enum PrSubcommand { /// Leaving this out will open your editor. #[clap(long)] body: Option, + /// The repo to create this issue on + #[clap(long, short)] + repo: Option, }, /// View the contents of a pull request View { /// The pull request to view. - id: u64, + id: IssueId, #[clap(subcommand)] command: Option, }, @@ -79,7 +83,7 @@ pub enum PrSubcommand { /// Add a comment on a pull request Comment { /// The pull request to comment on. - pr: u64, + pr: IssueId, /// The text content of the comment. /// /// Not including this in the command will open your editor. @@ -88,14 +92,14 @@ pub enum PrSubcommand { /// Edit the contents of a pull request Edit { /// The pull request to edit. - pr: u64, + pr: IssueId, #[clap(subcommand)] command: EditCommand, }, /// Close a pull request, without merging. Close { /// The pull request to close. - pr: u64, + pr: IssueId, /// A comment to add before closing. /// /// Adding without an argument will open your editor @@ -105,7 +109,7 @@ pub enum PrSubcommand { /// Merge a pull request Merge { /// The pull request to merge. - pr: u64, + pr: IssueId, /// The merge style to use. #[clap(long, short)] method: Option, @@ -118,7 +122,7 @@ pub enum PrSubcommand { /// The pull request to open in your browser. /// /// Leave this out to open the list of PRs. - id: Option, + id: Option, }, } @@ -241,32 +245,35 @@ pub enum ViewCommand { impl PrCommand { pub async fn run(self, keys: &crate::KeyInfo, host_name: Option<&str>) -> eyre::Result<()> { use PrSubcommand::*; - let repo = RepoInfo::get_current(host_name, self.repo.as_deref(), self.remote.as_deref())?; + let repo = RepoInfo::get_current(host_name, self.repo(), self.remote.as_deref())?; let api = keys.get_api(repo.host_url())?; - let repo = repo - .name() - .ok_or_eyre("couldn't get repo name, try specifying with --repo")?; + let repo = repo.name().ok_or_else(|| self.no_repo_error())?; match self.command { Create { title, base, head, body, + repo: _, } => create_pr(&repo, &api, title, base, head, body).await?, - Merge { pr, method, delete } => merge_pr(&repo, &api, pr, method, delete).await?, + Merge { pr, method, delete } => { + merge_pr(&repo, &api, pr.number, method, delete).await? + } View { id, command } => match command.unwrap_or(ViewCommand::Body) { - ViewCommand::Body => view_pr(&repo, &api, id).await?, + ViewCommand::Body => view_pr(&repo, &api, id.number).await?, ViewCommand::Comment { idx } => { - crate::issues::view_comment(&repo, &api, id, idx).await? + crate::issues::view_comment(&repo, &api, id.number, idx).await? } - ViewCommand::Comments => crate::issues::view_comments(&repo, &api, id).await?, - ViewCommand::Labels => view_pr_labels(&repo, &api, id).await?, + ViewCommand::Comments => { + crate::issues::view_comments(&repo, &api, id.number).await? + } + ViewCommand::Labels => view_pr_labels(&repo, &api, id.number).await?, ViewCommand::Diff { patch, editor } => { - view_diff(&repo, &api, id, patch, editor).await? + view_diff(&repo, &api, id.number, patch, editor).await? } - ViewCommand::Files => view_pr_files(&repo, &api, id).await?, + ViewCommand::Files => view_pr_files(&repo, &api, id.number).await?, ViewCommand::Commits { oneline } => { - view_pr_commits(&repo, &api, id, oneline).await? + view_pr_commits(&repo, &api, id.number, oneline).await? } }, Search { @@ -275,28 +282,96 @@ impl PrCommand { creator, assignee, state, + repo: _, } => view_prs(&repo, &api, query, labels, creator, assignee, state).await?, Edit { pr, command } => match command { EditCommand::Title { new_title } => { - crate::issues::edit_title(&repo, &api, pr, new_title).await? + crate::issues::edit_title(&repo, &api, pr.number, new_title).await? } EditCommand::Body { new_body } => { - crate::issues::edit_body(&repo, &api, pr, new_body).await? + crate::issues::edit_body(&repo, &api, pr.number, new_body).await? } EditCommand::Comment { idx, new_body } => { - crate::issues::edit_comment(&repo, &api, pr, idx, new_body).await? + crate::issues::edit_comment(&repo, &api, pr.number, idx, new_body).await? + } + EditCommand::Labels { add, rm } => { + edit_pr_labels(&repo, &api, pr.number, add, rm).await? } - EditCommand::Labels { add, rm } => edit_pr_labels(&repo, &api, pr, add, rm).await?, }, - Close { pr, with_msg } => crate::issues::close_issue(&repo, &api, pr, with_msg).await?, - Checkout { pr, branch_name } => { - checkout_pr(&repo, &api, pr, self.repo.is_some(), branch_name).await? + Close { pr, with_msg } => { + crate::issues::close_issue(&repo, &api, pr.number, with_msg).await? + } + Checkout { pr, branch_name } => checkout_pr(&repo, &api, pr, branch_name).await?, + Browse { id } => { + let number = id.as_ref().and_then(|s| { + let num_s = s.rsplit_once("#").map(|(_, b)| b).unwrap_or(s); + num_s.parse::().ok() + }); + browse_pr(&repo, &api, number).await? + } + Comment { pr, body } => { + crate::issues::add_comment(&repo, &api, pr.number, body).await? } - Browse { id } => browse_pr(&repo, &api, id).await?, - Comment { pr, body } => crate::issues::add_comment(&repo, &api, pr, body).await?, } Ok(()) } + + fn repo(&self) -> Option<&str> { + use PrSubcommand::*; + match &self.command { + Search { repo, .. } | Create { repo, .. } => repo.as_deref(), + Checkout { .. } => None, + View { id: pr, .. } + | Comment { pr, .. } + | Edit { pr, .. } + | Close { pr, .. } + | Merge { pr, .. } => pr.repo.as_deref(), + Browse { id } => id.as_ref().and_then(|s| { + let repo = s.rsplit_once("#").map(|(a, _)| a).unwrap_or(s); + // Don't treat a lone PR number as a repo name + if repo.parse::().is_ok() { + None + } else { + Some(repo) + } + }), + } + } + + fn no_repo_error(&self) -> eyre::Error { + use PrSubcommand::*; + match &self.command { + Search { .. } | Create { .. } => { + eyre::eyre!("can't figure what repo to access, try specifying with `--repo`") + } + Checkout { .. } => { + if git2::Repository::open(".").is_ok() { + eyre::eyre!("can't figure out what repo to access, try setting a remote tracking branch") + } else { + eyre::eyre!("pr checkout only works if the current directory is a git repo") + } + } + View { id: pr, .. } + | Comment { pr, .. } + | Edit { pr, .. } + | Close { pr, .. } + | Merge { pr, .. } => eyre::eyre!( + "can't figure out what repo to access, try specifying with `{{owner}}/{{repo}}#{}`", + pr.number + ), + Browse { id } => { + let number = id.as_ref().and_then(|s| { + let num_s = s.rsplit_once("#").map(|(_, b)| b).unwrap_or(s); + num_s.parse::().ok() + }); + if let Some(number) = number { + eyre::eyre!("can't figure out what repo to access, try specifying with `{{owner}}/{{repo}}#{}`", number) + } else { + eyre::eyre!("can't figure out what repo to access, try specifying with `{{owner}}/{{repo}}`") + } + } + } + } } pub async fn view_pr(repo: &RepoName, api: &Forgejo, id: u64) -> eyre::Result<()> { @@ -670,18 +745,8 @@ async fn checkout_pr( repo: &RepoName, api: &Forgejo, pr: PrNumber, - repo_specified: bool, branch_name: Option, ) -> eyre::Result<()> { - // this is so you don't checkout a pull request from an entirely different - // repository. i.e. in this repo I could run - // `fj pr -r codeberg.org/forgejo/forgejo checkout [num]` and have forgejo - // appear in this repo. - eyre::ensure!( - !repo_specified, - "Cannot checkout PR, `--repo` is not allowed when checking out a pull request" - ); - let local_repo = git2::Repository::open(".").unwrap(); let mut options = git2::StatusOptions::new();