@matheusbsbs / matheus-bagliano / commits / 46ba2abdfa

graph: remove `mapping_idx` and `graph_update_width()`

There's a duplication of logic between `graph_insert_into_new_columns()` and `graph_update_width()`. `graph_insert_into_new_columns()` is called repeatedly by `graph_update_columns()` with an `int *` that tracks the offset into the `mapping` array where we should write the next value. Each call to `graph_insert_into_new_columns()` effectively pushes one column index and one "null" value (-1) onto the `mapping` array and therefore increments `mapping_idx` by 2. `graph_update_width()` duplicates this process: the `width` of the graph is essentially the initial width of the `mapping` array before edges begin collapsing. The `graph_update_width()` function's logic effectively works out how many times `graph_insert_into_new_columns()` was called based on the relationship of the current commit to the rest of the graph. I'm about to make some changes that make the assignment of values into the `mapping` array more complicated. Rather than make `graph_update_width()` more complicated at the same time, we can simply remove this function and use `graph->width` to track the offset into the `mapping` array as we're building it. This removes the duplication and makes sure that `graph->width` is the same as the visual width of the `mapping` array once `graph_update_columns()` is complete. Signed-off-by: James Coglan <jcoglan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

James Coglan committed Oct 15, 2019 at 23:47 UTC 46ba2abdfa95a26a86714dab386a72a3a5b706a5

1 file changed +10 -55

graph.c

+10 -55

@@ -472,8 +472,7 @@ static int graph_find_new_column_by_commit(struct git_graph *graph,

472 }

473

474 static void graph_insert_into_new_columns(struct git_graph *graph,

475 - struct commit *commit,

476 - int *mapping_index)

475 + struct commit *commit)

476 {

477 int i = graph_find_new_column_by_commit(graph, commit);

478

@@ -487,50 +486,14 @@ static void graph_insert_into_new_columns(struct git_graph *graph,

486 graph->new_columns[i].color = graph_find_commit_color(graph, commit);

487 }

488

490 - graph->mapping[*mapping_index] = i;

491 - *mapping_index += 2;

492 -}

493 -

494 -static void graph_update_width(struct git_graph *graph,

495 - int is_commit_in_existing_columns)

496 -{

497 - /*

498 - * Compute the width needed to display the graph for this commit.

499 - * This is the maximum width needed for any row. All other rows

500 - * will be padded to this width.

501 - *

502 - * Compute the number of columns in the widest row:

503 - * Count each existing column (graph->num_columns), and each new

504 - * column added by this commit.

505 - */

506 - int max_cols = graph->num_columns + graph->num_parents;

507 -

508 - /*

509 - * Even if the current commit has no parents to be printed, it

510 - * still takes up a column for itself.

511 - */

512 - if (graph->num_parents < 1)

513 - max_cols++;

514 -

515 - /*

516 - * We added a column for the current commit as part of

517 - * graph->num_parents. If the current commit was already in

518 - * graph->columns, then we have double counted it.

519 - */

520 - if (is_commit_in_existing_columns)

521 - max_cols--;

522 -

523 - /*

524 - * Each column takes up 2 spaces

525 - */

526 - graph->width = max_cols * 2;

489 + graph->mapping[graph->width] = i;

490 + graph->width += 2;

491 }

492

493 static void graph_update_columns(struct git_graph *graph)

494 {

495 struct commit_list *parent;

496 int max_new_columns;

533 - int mapping_idx;

497 int i, seen_this, is_commit_in_columns;

498

499 /*

@@ -563,6 +526,8 @@ static void graph_update_columns(struct git_graph *graph)

526 for (i = 0; i < graph->mapping_size; i++)

527 graph->mapping[i] = -1;

528

529 + graph->width = 0;

530 +

531 /*

532 * Populate graph->new_columns and graph->mapping

533 *

@@ -573,7 +538,6 @@ static void graph_update_columns(struct git_graph *graph)

538 * supposed to end up after the collapsing is performed.

539 */

540 seen_this = 0;

576 - mapping_idx = 0;

541 is_commit_in_columns = 1;

542 for (i = 0; i <= graph->num_columns; i++) {

543 struct commit *col_commit;

@@ -587,7 +551,6 @@ static void graph_update_columns(struct git_graph *graph)

551 }

552

553 if (col_commit == graph->commit) {

590 - int old_mapping_idx = mapping_idx;

554 seen_this = 1;

555 graph->commit_index = i;

556 for (parent = first_interesting_parent(graph);

@@ -602,21 +565,18 @@ static void graph_update_columns(struct git_graph *graph)

565 !is_commit_in_columns) {

566 graph_increment_column_color(graph);

567 }

605 - graph_insert_into_new_columns(graph,

606 - parent->item,

607 - &mapping_idx);

568 + graph_insert_into_new_columns(graph, parent->item);

569 }

570 /*

610 - * We always need to increment mapping_idx by at

571 + * We always need to increment graph->width by at

572 * least 2, even if it has no interesting parents.

573 * The current commit always takes up at least 2

574 * spaces.

575 */

615 - if (mapping_idx == old_mapping_idx)

616 - mapping_idx += 2;

576 + if (graph->num_parents == 0)

577 + graph->width += 2;

578 } else {

618 - graph_insert_into_new_columns(graph, col_commit,

619 - &mapping_idx);

579 + graph_insert_into_new_columns(graph, col_commit);

580 }

581 }

582

@@ -626,11 +586,6 @@ static void graph_update_columns(struct git_graph *graph)

586 while (graph->mapping_size > 1 &&

587 graph->mapping[graph->mapping_size - 1] < 0)

588 graph->mapping_size--;

629 -

630 - /*

631 - * Compute graph->width for this commit

632 - */

633 - graph_update_width(graph, is_commit_in_columns);

589 }

590

591 void graph_update(struct git_graph *graph, struct commit *commit)