Skip to content

Commit 680dafd

Browse files
authored
prr: proper url parsing (#76)
Remove the regex URI hack in favor of URI-based parsing, to accomodate the more complete github URLs. Avoids me stripping a bunch of components to make the prr format happy; should fix #51
1 parent e5076af commit 680dafd

File tree

1 file changed

+64
-26
lines changed

1 file changed

+64
-26
lines changed

src/prr.rs

Lines changed: 64 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use serde_json::{json, Value};
1313

1414
use crate::parser::{FileComment, LineLocation, ReviewAction};
1515
use crate::review::{get_all_existing, Review, ReviewStatus};
16-
use regex::{Captures, Regex};
16+
use regex::Regex;
1717

1818
// Use lazy static to ensure regex is only compiled once
1919
lazy_static! {
@@ -22,12 +22,6 @@ lazy_static! {
2222
// danobi/prr-test-repo/6
2323
//
2424
static ref SHORT: Regex = Regex::new(r"^(?P<org>[\w\-_\.]+)/(?P<repo>[\w\-_\.]+)/(?P<pr_num>\d+)").unwrap();
25-
26-
// Regex for url input. Url looks something like:
27-
//
28-
// https://github.com/danobi/prr-test-repo/pull/6
29-
//
30-
static ref URL: Regex = Regex::new(r".*github\.com/(?P<org>.+)/(?P<repo>.+)/pull/(?P<pr_num>\d+)").unwrap();
3125
}
3226

3327
const GITHUB_BASE_URL: &str = "https://api.github.com";
@@ -227,21 +221,7 @@ impl Prr {
227221

228222
/// Parses a PR string in the form of `danobi/prr/24` and returns
229223
/// a tuple ("danobi", "prr", 24) or an error if string is malformed.
230-
/// If the local repository config is defined, it just needs the PR number.
231224
pub fn parse_pr_str(&self, s: &str) -> Result<(String, String, u64)> {
232-
let f = |captures: Captures| -> Result<(String, String, u64)> {
233-
let owner = captures.name("org").unwrap().as_str().to_owned();
234-
let repo = captures.name("repo").unwrap().as_str().to_owned();
235-
let pr_nr: u64 = captures
236-
.name("pr_num")
237-
.unwrap()
238-
.as_str()
239-
.parse()
240-
.context("Failed to parse pr number")?;
241-
242-
Ok((owner, repo, pr_nr))
243-
};
244-
245225
let repo = if let Some(local_config) = &self.config.local {
246226
if let Some(url) = &local_config.repository {
247227
if url.ends_with('/') {
@@ -257,12 +237,34 @@ impl Prr {
257237
};
258238

259239
if let Some(captures) = SHORT.captures(&repo) {
260-
f(captures)
261-
} else if let Some(captures) = URL.captures(&repo) {
262-
f(captures)
263-
} else {
264-
bail!("Invalid PR ref format")
240+
let owner = captures.name("org").unwrap().as_str().to_owned();
241+
let repo = captures.name("repo").unwrap().as_str().to_owned();
242+
let pr_nr: u64 = captures
243+
.name("pr_num")
244+
.unwrap()
245+
.as_str()
246+
.parse()
247+
.context("Failed to parse pr number")?;
248+
249+
return Ok((owner, repo, pr_nr));
265250
}
251+
252+
if repo.starts_with("http") || repo.contains("://") {
253+
let uri: Uri = repo.parse().context("Failed to parse URL")?;
254+
255+
let path = uri.path().trim_start_matches('/');
256+
let segments: Vec<_> = path.split('/').collect();
257+
258+
if segments.len() >= 4 && segments[2] == "pull" {
259+
let pr_num = segments[3]
260+
.parse::<u64>()
261+
.context("Failed to parse PR number")?;
262+
263+
return Ok((segments[0].to_string(), segments[1].to_string(), pr_num));
264+
}
265+
}
266+
267+
bail!("Invalid PR ref format")
266268
}
267269

268270
/// Gets a new review from the internet and writes it to the filesystem
@@ -637,6 +639,42 @@ mod tests {
637639
)
638640
}
639641

642+
#[tokio::test]
643+
async fn test_parse_github_url() {
644+
let pr_ref = "https://github.com/example/repo/pull/42";
645+
assert_eq!(
646+
PRR.0.parse_pr_str(pr_ref).unwrap(),
647+
("example".to_string(), "repo".to_string(), 42)
648+
)
649+
}
650+
651+
#[tokio::test]
652+
async fn test_parse_github_url_with_extra_path() {
653+
let pr_ref = "https://github.com/example/repo/pull/42/files";
654+
assert_eq!(
655+
PRR.0.parse_pr_str(pr_ref).unwrap(),
656+
("example".to_string(), "repo".to_string(), 42)
657+
)
658+
}
659+
660+
#[tokio::test]
661+
async fn test_parse_github_url_with_complex_path() {
662+
let pr_ref = "https://github.com/example/repo/pull/42/files/abc123..def456";
663+
assert_eq!(
664+
PRR.0.parse_pr_str(pr_ref).unwrap(),
665+
("example".to_string(), "repo".to_string(), 42)
666+
)
667+
}
668+
669+
#[tokio::test]
670+
async fn test_parse_custom_github_host() {
671+
let pr_ref = "https://github.acme.com/example/repo/pull/42";
672+
assert_eq!(
673+
PRR.0.parse_pr_str(pr_ref).unwrap(),
674+
("example".to_string(), "repo".to_string(), 42)
675+
)
676+
}
677+
640678
#[tokio::test]
641679
async fn test_local_config_repository() {
642680
let gconfig = r#"

0 commit comments

Comments
 (0)