From 662344b2e79d31bbbe6828cfa271eb84ac005d55 Mon Sep 17 00:00:00 2001 From: Brian Foley Date: Sun, 6 Sep 2026 14:00:22 +0100 Subject: [PATCH] editproject.php: Refactor row formatting not to use callables & echo This allows us to put proper arguments and type hints on all the formatting functions rather than forcing them all through a common interface, and the direct calls allow PHPStan to do typechecking. It also allows us to identify and remove a dead function. --- tools/project_manager/edit_common.inc | 251 +++++++++--------- .../edit_project_word_lists.php | 1 - tools/project_manager/editproject.php | 67 +++-- 3 files changed, 157 insertions(+), 162 deletions(-) diff --git a/tools/project_manager/edit_common.inc b/tools/project_manager/edit_common.inc index 1426e77fe..a7cc769cc 100644 --- a/tools/project_manager/edit_common.inc +++ b/tools/project_manager/edit_common.inc @@ -9,81 +9,81 @@ include_once($relPath.'User.inc'); include_once($relPath.'CharSuites.inc'); include_once($relPath.'Project.inc'); // load_image_sources() -function just_echo(string $field_value): void -{ - echo html_safe($field_value); -} - // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -/** @param array{maxlength?: int, required?: bool, type?: string, min?: int} $args */ -function text_field(?string $field_value, string $field_name, array $args = []): void -{ - $maxlength = $args["maxlength"] ?? null; +function format_text_field( + ?string $field_value, + string $field_name, + ?int $maxlength = null, + bool $required = false, + string $type = "text", + ?int $min = null +): string { $maxlength_attr = $maxlength ? "maxlength='$maxlength'" : ''; - $required = $args["required"] ?? false; $required_attr = $required ? " required" : ""; - $field_type = $args["type"] ?? "text"; - $min = $args["min"] ?? null; $min_attr = isset($min) ? " min='$min'" : ""; $enc_field_value = attr_safe($field_value); - echo ""; + return "" ; } + // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -/** @param array{maxlength?: int, required?: bool, type?: string, min?: int} $args */ -function DP_user_field(string $field_value, string $field_name, array $args = []): void -{ - $required = $args["required"] ?? false; +function format_DP_user_field( + string $field_value, + string $field_name, + bool $required = false, +): string { $required_attr = $required ? " required" : ""; $enc_field_value = attr_safe($field_value); - echo ""; + return ""; } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -function language_list(string $language): void +function format_language_list(string $language): string { $languages = Project::decode_language($language); $pri_language = $languages[0]; $sec_language = $languages[1] ?? ''; - echo html_safe(_("Languages are both shown to the user to select a project and used as dictionaries for WordCheck.")); - echo "
"; - echo html_safe(_("Languages without dictionaries will give the proofreader a warning that no dictionary exists in WordCheck.")); - echo "
"; - echo new_window_link("../../faq/wordcheck_data.php", _("WordCheck Site Data")); - - echo "

"; - echo html_safe(_("Primary")), ":
\n"; - echo html_safe(_("Secondary")), ": \n"; + return join("", [ + html_safe(_("Languages are both shown to the user to select a project and used as dictionaries for WordCheck.")), + "
", + html_safe(_("Languages without dictionaries will give the proofreader a warning that no dictionary exists in WordCheck.")), + "
", + new_window_link("../../faq/wordcheck_data.php", _("WordCheck Site Data")), + "

", + html_safe(_("Primary")), ":
\n", + html_safe(_("Secondary")), ": \n", + ]); } -function echo_language_options(string $default): void +function format_language_options(string $default): string { $langs_with_dicts = array_flip(get_languages_with_dictionaries()); ksort($langs_with_dicts); + $out = []; // output the languages with dictionaries first - echo ""; + $out[] = ""; foreach ($langs_with_dicts as $lang_name => $lang_code) { $selected_string = ($default == $lang_name) ? " selected" : ""; - echo "\n"; + $out[] = "\n"; } - echo ""; + $out[] = ""; // then everything else - echo ""; + $out[] = ""; foreach (get_iso_language_list() as $language) { $lang_name = $language['lang_name']; // skip the language if it was output in the group above @@ -91,27 +91,30 @@ function echo_language_options(string $default): void continue; } $selected_string = ($default == $lang_name) ? " selected" : ""; - echo "\n"; + $out[] = "\n"; } - echo ""; + $out[] = ""; + return join("", $out); } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -function genre_list(string $genre): void +function format_genre_list(string $genre): string { $genres = load_genre_translation_array(); - echo "\n"; + $out[] = format_placeholder_option($genre); foreach ($genres as $k => $v) { - echo ""; - echo "\n"; + $out[] = ">" . html_safe($v) . ""; + $out[] = "\n"; } - echo ""; + $out[] = ""; + return join("", $out); } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX @@ -120,7 +123,7 @@ function genre_list(string $genre): void * @param string[] $charsuites * @param CharSuite[] $project_charsuites */ -function charsuite_list(array $charsuites, array $project_charsuites): void +function format_charsuite_list(array $charsuites, array $project_charsuites): string { $enabled_charsuites = CharSuites::get_enabled(); @@ -137,19 +140,21 @@ function charsuite_list(array $charsuites, array $project_charsuites): void } ksort($all_charsuites); + $out = []; foreach ($all_charsuites as $k => $v) { - echo "
"; - echo "\n"; + $out[] = ">" . html_safe($v) . "
"; + $out[] = "\n"; } + return join("", $out); } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -function difficulty_list(string $difficulty_level): void +function format_difficulty_list(string $difficulty_level): string { global $pguser; @@ -160,18 +165,20 @@ function difficulty_list(string $difficulty_level): void unset($difficulty_list['beginner']); } + $out = []; foreach ($difficulty_list as $name => $label) { - echo ""; + $out[] = ">" . html_safe($label) . "     "; } + return join("", $out); } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -function special_list(string $special): void +function format_special_list(string $special): string { $special_days = load_special_days(); sort_special_days($special_days, "open_month,open_day"); @@ -187,82 +194,77 @@ function special_list(string $special): void $bdaymonth = 0; $bdayday = 0; + $out = []; + // drop down select box for which special day - echo ""; // add special case values first - echo ""; - echo "\n"; + $out[] = "\n"; - echo ""; - echo "\n"; + $out[] = ">" . html_safe(_("Birthday")) . "\n"; - echo ""; - echo "\n"; + $out[] = ">" . html_safe(_("Otherday")) . "\n"; // add the rest of the special days (the "ordinary" special days ;) ) foreach ($specials_array as $k => $v) { - echo ""; - echo "\n"; + $out[] = ">" . html_safe($v) . "\n"; } - echo ""; + $out[] = ""; - echo " " . new_window_link("show_specials.php", _("Special Days Info")); - echo "
"; + $out[] = " " . new_window_link("show_specials.php", _("Special Days Info")); + $out[] = "
"; // drop down selects for month and date, used for Birthday and Otherday specials - echo " ", html_safe(_("Birthday/Otherday: (month)")), " "; + $out[] = "\n"; $i = 1; while ($i <= 12) { $v = sprintf("%02d", $i); - echo ""; - echo "\n"; + $out[] = ">$v\n"; $i++; } - echo ""; + $out[] = ""; - echo " ", html_safe(_("(day)")), " "; + $out[] = "\n"; $i = 1; while ($i <= 31) { $v = sprintf("%02d", $i); - echo ""; - echo "\n"; + $out[] = ">$v\n"; $i++; } - echo ""; + $out[] = ""; + return join("", $out); } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -function image_source_list(string $image_source): void +function format_image_source_list(string $image_source): string { $imso_array = []; foreach (load_image_sources() as $code => $source) { @@ -273,66 +275,72 @@ function image_source_list(string $image_source): void } } + $out = []; // drop down select box for which image source - echo "\n"; + $out[] = format_placeholder_option($image_source); // add the pre-defined image_sources foreach ($imso_array as $k => $v) { - echo ""; - echo "\n"; + $out[] = ">" . html_safe($v) . ""; + $out[] = "\n"; } - echo " "; - echo "". html_safe(sprintf(_("'%1\$s Internal' if scanned by or for a %1\$s user."), SiteConfig::get()->site_abbreviation)).""; - echo " " . new_window_link("show_image_sources.php", _("Details of Image Sources")); - echo "
"; + $out[] = " "; + $out[] = "". html_safe(sprintf(_("'%1\$s Internal' if scanned by or for a %1\$s user."), SiteConfig::get()->site_abbreviation)).""; + $out[] = " " . new_window_link("show_image_sources.php", _("Details of Image Sources")); + $out[] = "
"; + return join("", $out); } -function maybe_echo_placeholder_option(string $initial): void +function format_placeholder_option(string $initial): string { if ("" == $initial) { // none of the real options can have this value - echo "\n"; + return "\n"; + } else { + return ""; } } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX /** @param 'markdown'|'html' $comment_format */ -function proj_comments_format(string $comment_format): void +function format_proj_comments_format(string $comment_format): string { $comment_format_list = [ 'markdown' => new_window_link('https://www.pgdp.net/wiki/Markdown', 'Markdown'), 'html' => "HTML", ]; + $out = []; foreach ($comment_format_list as $name => $label) { - echo ""; + $out[] = ">"; + $out[] = "$label     "; } + return join("", $out); } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -function proj_comments_field(string $comments, ?string $_field_name): void +function format_proj_comments_field(string $comments): string { $enc_comments = html_safe($comments); - echo ""; + return ""; } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -function word_lists(?string $_value, ?string $_fieldname, string $projectid): void +function format_word_lists(string $projectid): string { - echo new_window_link( + return new_window_link( "edit_project_word_lists.php?projectid=$projectid", _("Edit project word lists") ); @@ -340,20 +348,11 @@ function word_lists(?string $_value, ?string $_fieldname, string $projectid): vo // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX -function extra_credits_field(string $extra_credits): void +function format_extra_credits_field(string $extra_credits): string { $enc_credits = html_safe($extra_credits); - echo ""; + return ""; } // XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX - - - -function description_field(string $description, string $field_name): void -{ - $enc_description = html_safe($description); - - echo ""; -} diff --git a/tools/project_manager/edit_project_word_lists.php b/tools/project_manager/edit_project_word_lists.php index 522724ee6..32116adea 100644 --- a/tools/project_manager/edit_project_word_lists.php +++ b/tools/project_manager/edit_project_word_lists.php @@ -3,7 +3,6 @@ include_once($relPath.'base.inc'); include_once($relPath.'theme.inc'); include_once($relPath.'links.inc'); -include_once('edit_common.inc'); include_once($relPath.'wordcheck_engine.inc'); include_once($relPath.'metarefresh.inc'); include_once($relPath.'Project.inc'); diff --git a/tools/project_manager/editproject.php b/tools/project_manager/editproject.php index 44f6b58c6..94453c223 100644 --- a/tools/project_manager/editproject.php +++ b/tools/project_manager/editproject.php @@ -478,16 +478,17 @@ public function show_visible_controls(): void { global $pguser; + $site = SiteConfig::get()->site_abbreviation; $can_set_difficulty_tofrom_beginner = ($pguser == "BEGIN") || user_is_a_sitemanager(); $can_edit_PPer = true; $is_checked_out = false; if (!empty($this->project->projectid)) { - $this->row(_("Project ID"), 'just_echo', $this->project->projectid); + $this->row(_("Project ID"), html_safe($this->project->projectid)); // do some things that depend on the project state if ($this->project->state == PROJ_DELETE) { - $this->row(_("Reason for Deletion"), 'text_field', $this->project->deletion_reason, 'deletion_reason'); + $this->row(_("Reason for Deletion"), format_text_field($this->project->deletion_reason, 'deletion_reason')); } elseif ($this->project->state == PROJ_POST_FIRST_CHECKED_OUT) { // once the project is in PP, PPer can only be changed by an SA, PF, // or if it's checked out to the PM @@ -500,70 +501,66 @@ public function show_visible_controls(): void $can_edit_PPer = user_is_a_sitemanager(); } } - $this->row(_("Title"), 'text_field', $this->project->nameofwork, 'nameofwork', '', ["maxlength" => 255, "required" => true]); - $this->row(_("Author"), 'text_field', $this->project->authorsname, 'authorsname', '', ["maxlength" => 255, "required" => true]); + $this->row(_("Title"), format_text_field($this->project->nameofwork, 'nameofwork', maxlength: 255, required: true)); + $this->row(_("Author"), format_text_field($this->project->authorsname, 'authorsname', maxlength: 255, required: true)); if (user_is_a_sitemanager()) { // SAs are the only ones who can change this - $this->row(_("Project Manager"), 'DP_user_field', $this->project->username, 'username', sprintf(_("%s username only."), SiteConfig::get()->site_abbreviation), ["required" => true]); + $explain = sprintf(_("%s username only."), $site); + $this->row(_("Project Manager"), format_DP_user_field($this->project->username, 'username', required: true), $explain); } - $this->row(_("Language"), 'language_list', $this->project->language); + $this->row(_("Language"), format_language_list($this->project->language)); $project_charsuites = []; if (isset($this->project->projectid)) { $project_charsuites = $this->project->get_charsuites(false); } - $this->row(_("Character Suites"), 'charsuite_list', $this->charsuites, $project_charsuites); - $this->row(_("Custom Characters"), 'text_field', $this->project->custom_chars, 'custom_chars'); + $this->row(_("Character Suites"), format_charsuite_list($this->charsuites, $project_charsuites)); + $this->row(_("Custom Characters"), format_text_field($this->project->custom_chars, 'custom_chars')); - $this->row(_("Genre"), 'genre_list', $this->project->genre); + $this->row(_("Genre"), format_genre_list($this->project->genre)); if ($this->project->difficulty == "beginner" && !$can_set_difficulty_tofrom_beginner) { // allow PF to edit a BEGIN project, but without altering the difficulty - $this->row(_("Difficulty"), 'just_echo', _("Beginner")); + $this->row(_("Difficulty"), _("Beginner")); echo ""; } else { - $this->row(_("Difficulty"), 'difficulty_list', $this->project->difficulty); + $this->row(_("Difficulty"), format_difficulty_list($this->project->difficulty)); } - $this->row(_("Special Day"), 'special_list', $this->project->special_code); + $this->row(_("Special Day"), format_special_list($this->project->special_code)); if ($can_edit_PPer) { - $this->row(_("PPer/PPVer"), 'DP_user_field', $this->project->checkedoutby, 'checkedoutby', sprintf(_("Optionally reserve for a PPer. %s username only."), SiteConfig::get()->site_abbreviation)); + $explain = sprintf(_("Optionally reserve for a PPer. %s username only."), $site); + $this->row(_("PPer/PPVer"), format_DP_user_field($this->project->checkedoutby, 'checkedoutby'), $explain); } else { - $this->row(_("PPer/PPVer"), 'just_echo', $this->project->checkedoutby); + $this->row(_("PPer/PPVer"), html_safe($this->project->checkedoutby)); echo ""; } - $this->row(_("Image Source"), 'image_source_list', $this->project->image_source); - $this->row(_("Image Preparer"), 'DP_user_field', $this->project->image_preparer, 'image_preparer', sprintf(_("%s user who scanned or harvested the images."), SiteConfig::get()->site_abbreviation)); - $this->row(_("Text Preparer"), 'DP_user_field', $this->project->text_preparer, 'text_preparer', sprintf(_("%s user who prepared the text files."), SiteConfig::get()->site_abbreviation)); + $this->row(_("Image Source"), format_image_source_list($this->project->image_source)); + $explain = sprintf(_("%s user who scanned or harvested the images."), $site); + $this->row(_("Image Preparer"), format_DP_user_field($this->project->image_preparer, 'image_preparer'), $explain); + $explain = sprintf(_("%s user who prepared the text files."), $site); + $this->row(_("Text Preparer"), format_DP_user_field($this->project->text_preparer, 'text_preparer'), $explain); $this->row( _("Extra Credits
(to be included in list of names--no URLs)"), - 'extra_credits_field', - $this->project->extra_credits, - null, - '', - [], - true + format_extra_credits_field($this->project->extra_credits), + html_label: true ); if ($this->project->scannercredit != '') { - $this->row(_("Scanner Credit (deprecated)"), 'text_field', $this->project->scannercredit, 'scannercredit'); + $this->row(_("Scanner Credit (deprecated)"), format_text_field($this->project->scannercredit, 'scannercredit')); } - $this->row(_("Clearance Line"), 'text_field', $this->project->clearance, 'clearance'); - $this->row(_("PG etext number"), 'text_field', $this->project->postednum, 'postednum', '', ["type" => "number", "min" => 1]); - $this->row(_("Project Comments Format"), 'proj_comments_format', $this->project->comment_format); - $this->row(_("Project Comments"), 'proj_comments_field', $this->project->comments); + $this->row(_("Clearance Line"), format_text_field($this->project->clearance, 'clearance')); + $this->row(_("PG etext number"), format_text_field((string)$this->project->postednum, 'postednum', type: "number", min: 1)); + $this->row(_("Project Comments Format"), format_proj_comments_format($this->project->comment_format)); + $this->row(_("Project Comments"), format_proj_comments_field($this->project->comments)); // don't show the word list line if we're in the process of cloning if (!empty($this->project->projectid)) { - $this->row(_("Project Dictionary"), 'word_lists', null, null, '', $this->project->projectid); + $this->row(_("Project Dictionary"), format_word_lists($this->project->projectid)); } } - public function row( string $label, - string $display_function, - mixed $field_value, - mixed $field_name = null, + string $display, string $explain = '', - mixed $args = [], bool $html_label = false ): void { echo ""; @@ -571,7 +568,7 @@ public function row( echo $html_label ? $label : html_safe($label); echo ""; echo ""; - $display_function($field_value, $field_name, $args); + echo $display; echo " "; echo html_safe($explain); echo "";