handle_revision_arg: add handle_dotdot() helper
The handle_revision_arg function is rather long, and a big chunk of it is handling the range operators. Let's pull that out to a separate helper. While we're doing so, we can clean up a few of the rough edges that made the flow hard to follow: - instead of manually restoring *dotdot (that we overwrote with a NUL), do the real work in a sub-helper, which makes it clear that the munge/restore lines are a matched pair - eliminate a goto which wasn't actually used for control flow, but only to avoid duplicating a few lines (instead, those lines are pushed into another helper function) - use early returns instead of deep nesting - consistently name all variables for the left-hand side of the range as "a" (rather than "this" or "from") and the right-hand side as "b" (rather than "next", or using the unadorned "sha1" or "flags" from the main function). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
May 19, 2017 at 08:52 UTC
62faad5aa5441942c333c15f3afa03a6e2dd994d
1 file changed
+102
-75
revision.c
+102
-75
@@ -1429,10 +1429,109 @@ static void prepare_show_merge(struct rev_info *revs)
1429
revs->limited = 1;
1430
}
1431
1432
+static int dotdot_missing(const char *arg, char *dotdot,
1433
+ struct rev_info *revs, int symmetric)
1434
+{
1435
+ if (revs->ignore_missing)
1436
+ return 0;
1437
+ /* de-munge so we report the full argument */
1438
+ *dotdot = '.';
1439
+ die(symmetric
1440
+ ? "Invalid symmetric difference expression %s"
1441
+ : "Invalid revision range %s", arg);
1442
+}
1443
+
1444
+static int handle_dotdot_1(const char *arg, char *dotdot,
1445
+ struct rev_info *revs, int flags,
1446
+ int cant_be_filename)
1447
+{
1448
+ const char *a_name, *b_name;
1449
+ struct object_id a_oid, b_oid;
1450
+ struct object *a_obj, *b_obj;
1451
+ unsigned int a_flags, b_flags;
1452
+ int symmetric = 0;
1453
+ unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);
1454
+
1455
+ a_name = arg;
1456
+ if (!*a_name)
1457
+ a_name = "HEAD";
1458
+
1459
+ b_name = dotdot + 2;
1460
+ if (*b_name == '.') {
1461
+ symmetric = 1;
1462
+ b_name++;
1463
+ }
1464
+ if (!*b_name)
1465
+ b_name = "HEAD";
1466
+
1467
+ if (get_sha1_committish(a_name, a_oid.hash) ||
1468
+ get_sha1_committish(b_name, b_oid.hash))
1469
+ return -1;
1470
+
1471
+ if (!cant_be_filename) {
1472
+ *dotdot = '.';
1473
+ verify_non_filename(revs->prefix, arg);
1474
+ *dotdot = '\0';
1475
+ }
1476
+
1477
+ a_obj = parse_object(a_oid.hash);
1478
+ b_obj = parse_object(b_oid.hash);
1479
+ if (!a_obj || !b_obj)
1480
+ return dotdot_missing(arg, dotdot, revs, symmetric);
1481
+
1482
+ if (!symmetric) {
1483
+ /* just A..B */
1484
+ b_flags = flags;
1485
+ a_flags = flags_exclude;
1486
+ } else {
1487
+ /* A...B -- find merge bases between the two */
1488
+ struct commit *a, *b;
1489
+ struct commit_list *exclude;
1490
+
1491
+ a = lookup_commit_reference(a_obj->oid.hash);
1492
+ b = lookup_commit_reference(b_obj->oid.hash);
1493
+ if (!a || !b)
1494
+ return dotdot_missing(arg, dotdot, revs, symmetric);
1495
+
1496
+ exclude = get_merge_bases(a, b);
1497
+ add_rev_cmdline_list(revs, exclude, REV_CMD_MERGE_BASE,
1498
+ flags_exclude);
1499
+ add_pending_commit_list(revs, exclude, flags_exclude);
1500
+ free_commit_list(exclude);
1501
+
1502
+ b_flags = flags;
1503
+ a_flags = flags | SYMMETRIC_LEFT;
1504
+ }
1505
+
1506
+ a_obj->flags |= a_flags;
1507
+ b_obj->flags |= b_flags;
1508
+ add_rev_cmdline(revs, a_obj, a_name, REV_CMD_LEFT, a_flags);
1509
+ add_rev_cmdline(revs, b_obj, b_name, REV_CMD_RIGHT, b_flags);
1510
+ add_pending_object(revs, a_obj, a_name);
1511
+ add_pending_object(revs, b_obj, b_name);
1512
+ return 0;
1513
+}
1514
+
1515
+static int handle_dotdot(const char *arg,
1516
+ struct rev_info *revs, int flags,
1517
+ int cant_be_filename)
1518
+{
1519
+ char *dotdot = strstr(arg, "..");
1520
+ int ret;
1521
+
1522
+ if (!dotdot)
1523
+ return -1;
1524
+
1525
+ *dotdot = '\0';
1526
+ ret = handle_dotdot_1(arg, dotdot, revs, flags, cant_be_filename);
1527
+ *dotdot = '.';
1528
+
1529
+ return ret;
1530
+}
1531
+
1532
int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsigned revarg_opt)
1533
{
1534
struct object_context oc;
1435
- char *dotdot;
1535
char *mark;
1536
struct object *object;
1537
unsigned char sha1[20];
@@ -1451,80 +1550,8 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
1550
return -1;
1551
}
1552
1454
- dotdot = strstr(arg, "..");
1455
- if (dotdot) {
1456
- unsigned char from_sha1[20];
1457
- const char *next = dotdot + 2;
1458
- const char *this = arg;
1459
- int symmetric = *next == '.';
1460
- unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);
1461
- unsigned int a_flags;
1462
-
1463
- *dotdot = 0;
1464
- next += symmetric;
1465
-
1466
- if (!*next)
1467
- next = "HEAD";
1468
- if (dotdot == arg)
1469
- this = "HEAD";
1470
- if (!get_sha1_committish(this, from_sha1) &&
1471
- !get_sha1_committish(next, sha1)) {
1472
- struct object *a_obj, *b_obj;
1473
-
1474
- if (!cant_be_filename) {
1475
- *dotdot = '.';
1476
- verify_non_filename(revs->prefix, arg);
1477
- *dotdot = '\0';
1478
- }
1479
-
1480
- a_obj = parse_object(from_sha1);
1481
- b_obj = parse_object(sha1);
1482
- if (!a_obj || !b_obj) {
1483
- missing:
1484
- *dotdot = '.';
1485
- if (revs->ignore_missing)
1486
- return 0;
1487
- die(symmetric
1488
- ? "Invalid symmetric difference expression %s"
1489
- : "Invalid revision range %s", arg);
1490
- }
1491
-
1492
- if (!symmetric) {
1493
- /* just A..B */
1494
- a_flags = flags_exclude;
1495
- } else {
1496
- /* A...B -- find merge bases between the two */
1497
- struct commit *a, *b;
1498
- struct commit_list *exclude;
1499
-
1500
- a = lookup_commit_reference(a_obj->oid.hash);
1501
- b = lookup_commit_reference(b_obj->oid.hash);
1502
- if (!a || !b)
1503
- goto missing;
1504
- exclude = get_merge_bases(a, b);
1505
- add_rev_cmdline_list(revs, exclude,
1506
- REV_CMD_MERGE_BASE,
1507
- flags_exclude);
1508
- add_pending_commit_list(revs, exclude,
1509
- flags_exclude);
1510
- free_commit_list(exclude);
1511
-
1512
- a_flags = flags | SYMMETRIC_LEFT;
1513
- }
1514
-
1515
- a_obj->flags |= a_flags;
1516
- b_obj->flags |= flags;
1517
- add_rev_cmdline(revs, a_obj, this,
1518
- REV_CMD_LEFT, a_flags);
1519
- add_rev_cmdline(revs, b_obj, next,
1520
- REV_CMD_RIGHT, flags);
1521
- add_pending_object(revs, a_obj, this);
1522
- add_pending_object(revs, b_obj, next);
1523
- *dotdot = '.';
1524
- return 0;
1525
- }
1526
- *dotdot = '.';
1527
- }
1553
+ if (!handle_dotdot(arg, revs, flags, revarg_opt))
1554
+ return 0;
1555
1556
mark = strstr(arg, "^@");
1557
if (mark && !mark[2]) {