Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle r? as assignment #433

Closed
wants to merge 1 commit into from
Closed

Handle r? as assignment #433

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 115 additions & 39 deletions parser/src/command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,43 +58,98 @@ impl<'a> Input<'a> {
}
}

pub fn parse_command(&mut self) -> Command<'a> {
let start = match find_commmand_start(&self.all[self.parsed..], self.bot) {
fn maybe_parse_review(&mut self) -> Option<(Tokenizer<'a>, assign::AssignCommand)> {
// Both `r?` and `R?`.
let start = match self.all[self.parsed..]
.find("r?")
.or_else(|| self.all[self.parsed..].find("R?"))
{
Some(pos) => pos,
None => return Command::None,
None => return None,
};
self.parsed += start;
let mut tok = Tokenizer::new(&self.all[self.parsed..]);
assert_eq!(
tok.next_token().unwrap(),
Some(Token::Word(&format!("@{}", self.bot)))
);
log::info!("identified potential command");
match tok.next_token() {
Ok(Some(Token::Word(w))) => {
if w != "r" && w != "R" {
// If we're intersecting with something else just exit
log::trace!("received odd review start token: {:?}", w);
return None;
}
}
other => {
log::trace!("received odd review start token: {:?}", other);
return None;
}
}
match tok.next_token() {
Ok(Some(Token::Question)) => {}
other => {
log::trace!("received odd review start token: {:?}", other);
return None;
}
}
log::info!("identified potential review request");
match tok.next_token() {
Ok(Some(Token::Word(w))) => {
let mentions = crate::mentions::get_mentions(w);
if let [a] = &mentions[..] {
return Some((
tok,
assign::AssignCommand::User {
username: (*a).to_owned(),
},
));
} else {
log::trace!("{:?} had non-one mention: {:?}", w, mentions);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be useful to allow more than a person to be assigned: occasionally I write r? @foo @bar and manually assign @bar once highfive assigns @foo. Highfive was written before GitHub allowed multiple assignees, so I guess that's the reason why highfive didn't support that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I largely agree, though I think it is pretty useful to have just one person assigned at a time -- then there's less "well they'll do it" :)

But I don't want to try to make things better before we make things work; migrating things to triagebot is the first priority, and then once that's done we can enhance.

None
}
}
other => {
log::trace!("received odd review start token: {:?}", other);
None
}
}
}

pub fn parse_command(&mut self) -> Command<'a> {
let mut success = vec![];

let original_tokenizer = tok.clone();

success.extend(parse_single_command(
relabel::RelabelCommand::parse,
Command::Relabel,
&original_tokenizer,
));
success.extend(parse_single_command(
assign::AssignCommand::parse,
Command::Assign,
&original_tokenizer,
));
success.extend(parse_single_command(
ping::PingCommand::parse,
Command::Ping,
&original_tokenizer,
));
success.extend(parse_single_command(
nominate::NominateCommand::parse,
Command::Nominate,
&original_tokenizer,
));
if let Some((tok, assign)) = self.maybe_parse_review() {
success.push((tok, Command::Assign(Ok(assign))));
}

if let Some(start) = find_commmand_start(&self.all[self.parsed..], self.bot) {
self.parsed += start;
let mut tok = Tokenizer::new(&self.all[self.parsed..]);
assert_eq!(
tok.next_token().unwrap(),
Some(Token::Word(&format!("@{}", self.bot)))
);
log::info!("identified potential command");
let original_tokenizer = tok.clone();

success.extend(parse_single_command(
relabel::RelabelCommand::parse,
Command::Relabel,
&original_tokenizer,
));
success.extend(parse_single_command(
assign::AssignCommand::parse,
Command::Assign,
&original_tokenizer,
));
success.extend(parse_single_command(
ping::PingCommand::parse,
Command::Ping,
&original_tokenizer,
));
success.extend(parse_single_command(
nominate::NominateCommand::parse,
Command::Nominate,
&original_tokenizer,
));
}

if success.len() > 1 {
panic!(
Expand All @@ -104,17 +159,16 @@ impl<'a> Input<'a> {
);
}

if self
.code
.overlaps_code((self.parsed)..(self.parsed + tok.position()))
.is_some()
{
log::info!("command overlaps code; code: {:?}", self.code);
return Command::None;
}

match success.pop() {
Some((mut tok, c)) => {
if self
.code
.overlaps_code((self.parsed)..(self.parsed + tok.position()))
.is_some()
{
log::info!("command overlaps code; code: {:?}", self.code);
return Command::None;
}
// if we errored out while parsing the command do not move the input forwards
if c.is_ok() {
self.parsed += tok.position();
Expand Down Expand Up @@ -190,3 +244,25 @@ fn move_input_along_1() {
// don't move input along if parsing the command fails
assert_eq!(input.parsed, 0);
}

#[test]
fn parse_assign_review() {
let input = "R? @user";
let mut input = Input::new(input, "bot");
match input.parse_command() {
Command::Assign(Ok(x)) => assert_eq!(
x,
assign::AssignCommand::User {
username: String::from("user"),
}
),
o => panic!("unknown: {:?}", o),
};
}

#[test]
fn parse_assign_review_no_panic() {
let input = "R ?";
let mut input = Input::new(input, "bot");
assert!(input.parse_command().is_none());
}
10 changes: 3 additions & 7 deletions src/handlers/assign.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,10 @@
//!
//! We need to fake-assign ourselves and add a 'claimed by' section to the top-level comment.
//!
//! Such assigned issues should also be placed in a queue to ensure that the user remains
//! active; the assigned user will be asked for a status report every 2 weeks (XXX: timing).
//!
//! If we're intending to ask for a status report but no comments from the assigned user have
//! been given for the past 2 weeks, the bot will de-assign the user. They can once more claim
//! the issue if necessary.
//!
//! Assign users with `@rustbot assign @gh-user` or `@rustbot claim` (self-claim).
//!
//! This also gets invoked by `r?` in comments, with equivalent handling to
//! `rustbot assign @gh-user`.

use crate::{
config::AssignConfig,
Expand Down