Return partial differential fields when there's an error parsing

Summary:
If someone typos one name in a cc or reviewer list, it would be nice if we
display all of the valid names in the field when running arc diff (in addition
to the error message).

Test Plan:
used the conduit console to check that calling differential.parsecommitmessage
with a list of some valid and some invalid ccs returns a result with both an
error and a list of some ccs. Also ran arc diff with that list of ccs to check
for the correct user experience.

Reviewers: epriestley, jungejason, vrana

Reviewed By: epriestley

CC: aran, Koolvin

Differential Revision: https://secure.phabricator.com/D2542
This commit is contained in:
Nick Harper
2012-05-22 17:47:12 -07:00
parent ccd37afab8
commit e4e56bb431
4 changed files with 18 additions and 5 deletions

View File

@@ -69,11 +69,12 @@ final class ConduitAPI_differential_parsecommitmessage_Method
$field = $aux_fields[$field_key]; $field = $aux_fields[$field_key];
try { try {
$fields[$field_key] = $field->parseValueFromCommitMessage($field_value); $fields[$field_key] = $field->parseValueFromCommitMessage($field_value);
$field->setValueFromParsedCommitMessage($fields[$field_key]);
} catch (DifferentialFieldParseException $ex) { } catch (DifferentialFieldParseException $ex) {
$field_label = $field->renderLabelForCommitMessage(); $field_label = $field->renderLabelForCommitMessage();
$errors[] = "Error parsing field '{$field_label}': ".$ex->getMessage(); $errors[] = "Error parsing field '{$field_label}': ".$ex->getMessage();
$fields[$field_key] = $ex->getPartialParse();
} }
$field->setValueFromParsedCommitMessage($fields[$field_key]);
} }
if (!$is_partial) { if (!$is_partial) {

View File

@@ -1,7 +1,7 @@
<?php <?php
/* /*
* Copyright 2011 Facebook, Inc. * Copyright 2012 Facebook, Inc.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@@ -18,4 +18,14 @@
final class DifferentialFieldParseException extends Exception { final class DifferentialFieldParseException extends Exception {
private $partialParse;
public function __construct($message, $partial_parse = null) {
parent::__construct($message);
$this->partialParse = $partial_parse;
}
public function getPartialParse() {
return $this->partialParse;
}
} }

View File

@@ -750,7 +750,8 @@ abstract class DifferentialFieldSpecification {
? "users and mailing lists" ? "users and mailing lists"
: "users"; : "users";
throw new DifferentialFieldParseException( throw new DifferentialFieldParseException(
"Commit message references nonexistent {$what}: {$invalid}."); "Commit message references nonexistent {$what}: {$invalid}.",
array_unique($results));
} }
return array_unique($results); return array_unique($results);

View File

@@ -1,7 +1,7 @@
<?php <?php
/* /*
* Copyright 2011 Facebook, Inc. * Copyright 2012 Facebook, Inc.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@@ -149,7 +149,8 @@ final class DifferentialManiphestTasksFieldSpecification
} }
$invalid = implode(', ', $invalid); $invalid = implode(', ', $invalid);
throw new DifferentialFieldParseException( throw new DifferentialFieldParseException(
"Commit message references nonexistent {$what}: {$invalid}."); "Commit message references nonexistent {$what}: {$invalid}.",
$task_phids);
} }
return $task_phids; return $task_phids;