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

Link "skip empty diffs" with "without formatting" #1114

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Link "skip empty diffs" with "without formatting" #1114

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
57 changes: 45 additions & 12 deletions tools/project_manager/diff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,42 @@ function copyToClip(textstring) {
echo $navigation_text;
}

// Load an associative array for which pages in the project have diffs.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since we're trying to get better about this could we add

/**
  * @return array<string, array<string, mixed>> 
  * Return an associative array indexed by page image name, where each page has an array of attributes.
*/
function load_page_diff_array(Project $project, Round $L_round, Round $R_round, bool $include_no_format = false): array

function load_page_diff_array(Project $project, $L_round, $R_round, bool $include_no_format = false): array
{
if ($include_no_format) {
$un_formatter = new PageUnformatter();
$text_columns = "
$L_round->text_column_name as L_text,
$R_round->text_column_name as R_text,
";
} else {
$text_columns = "";
}
$sql = "
SELECT image,
$L_round->user_column_name as username,
$text_columns
CAST($L_round->text_column_name AS BINARY) = CAST($R_round->text_column_name AS BINARY) AS is_same
FROM $project->projectid
ORDER BY image ASC
";
$res = DPDatabase::query($sql);
$result = [];
while ($row = mysqli_fetch_assoc($res)) {
$result[$row["image"]] = [
"username" => $row["username"],
"is_diff" => ! $row["is_same"],
];
if ($include_no_format) {
$L_text = $un_formatter->remove_formatting($row["L_text"], false);
$R_text = $un_formatter->remove_formatting($row["R_text"], false);
$result[$row["image"]]["is_diff_without_formatting"] = $L_text != $R_text;
}
}
return $result;
}

/**
* Build up the text for the navigation bit, so we can repeat it
* again at the bottom of the page
Expand Down Expand Up @@ -199,27 +235,24 @@ function get_navigation(
}
$navigation_text .= "\n" . _("Jump to") . ": <select name='jumpto' onChange='$jump_to_js'>\n";

$query = "
SELECT image,
$L_round->user_column_name,
CAST($L_round->text_column_name AS BINARY) = CAST($R_round->text_column_name AS BINARY) AS is_empty_diff
FROM $project->projectid
ORDER BY image ASC
";
$res = DPDatabase::query($query);
$diff_array = load_page_diff_array($project, $L_round, $R_round, $format == "remove");

$prev_image = "";
$next_image = "";
$prev_from_proofer = "";
$next_from_proofer = "";
$got_there = false;
// construct the dropdown; work out where previous and next buttons should take us
while ([$this_val, $this_user, $is_empty_diff] = mysqli_fetch_row($res)) {
foreach ($diff_array as $this_val => $diff_record) {
$this_user = $diff_record["username"];
$is_diff = $format == "remove" ? $diff_record["is_diff_without_formatting"] : $diff_record["is_diff"];
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you wanted to be slightly cleverer you could push the ternary inside the index:
$is_diff == $diff_record[$format == "remove" ? "is_diff_without_formatting": "is_diff"];
or

$idx = $format == "remove" ? "is_diff_without_formatting": "is_diff";
$is_diff = $diff_record[$idx];


$navigation_text .= "\n<option value='$this_val'";

if ($this_val == $image) {
$navigation_text .= " selected"; // make the correct element of the drop down selected
$got_there = true;
} elseif ($only_nonempty_diffs && $is_empty_diff) {
} elseif ($only_nonempty_diffs && !$is_diff) {
$navigation_text .= " disabled"; // Disable empty diffs in the dropdown and skip the other checks
} elseif ($got_there) {
// we are at the one after the current one
Expand Down Expand Up @@ -270,9 +303,9 @@ function get_navigation(
}

$checked_attribute = $only_nonempty_diffs ? 'checked' : '';

$checkbox_title = $format == "remove" ? _('Skip empty diffs, ignoring formatting') : _('Skip empty diffs');
$navigation_text .= "\n<input type='checkbox' name='only_nonempty_diffs' $checked_attribute id='only_nonempty_diffs' onclick='this.form.submit()'>\n";
$navigation_text .= "\n<label for='only_nonempty_diffs'>" . html_safe(_('Skip empty diffs')) . "</label>\n";
$navigation_text .= "\n<label for='only_nonempty_diffs'>" . html_safe($checkbox_title) . "</label>\n";
$navigation_text .= "\n</form>\n";

return $navigation_text;
Expand Down