Support system messages for upload text #60
Conversation
docs: add default 'simplebatchupload-parameters' system message feat: support unordered list in customization settings feat: support system message on default upload text feat: support unnamed on parser function call
📝 WalkthroughWalkthroughThis pull request introduces Chinese language aliases for SimpleBatchUpload, adds two new translation keys with documentation, refactors the parameter provider's message lookup flow with dynamic fallback logic, and adjusts parameter handling in the upload button renderer to process parameters unconditionally. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/SimpleBatchUpload.alias.php (1)
2-23: Minor: File header references "ExtensionManager" instead of "SimpleBatchUpload".The file header comment and
@ingrouptag reference "ExtensionManager" (lines 3, 22), which appears to be a copy-paste artifact from another extension. Consider updating these to reference "SimpleBatchUpload" for accuracy.src/ParameterProvider.php (1)
98-98: Complex one-liner could benefit from decomposition for readability.This line performs multiple operations: strips HTML comment, explodes into lines, then strips bullet prefixes. While functionally correct (PHP's
preg_replacehandles arrays), this could be clearer:Optional refactor for readability
- $paramLines = preg_replace( '/^\*\s*/', '', explode( "\n", preg_replace( '/^[\s\n]*<!--[\s\S]*?-->[\s\n]*/', '', $paramMsg->plain() ) ) ); + $rawText = $paramMsg->plain(); + $textWithoutComment = preg_replace( '/^[\s\n]*<!--[\s\S]*?-->[\s\n]*/', '', $rawText ); + $lines = explode( "\n", $textWithoutComment ); + $paramLines = preg_replace( '/^\*\s*/', '', $lines );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
i18n/en.jsoni18n/qqq.jsonsrc/ParameterProvider.phpsrc/SimpleBatchUpload.alias.phpsrc/UploadButtonRenderer.php
🧰 Additional context used
🧬 Code graph analysis (1)
src/UploadButtonRenderer.php (1)
src/ParameterProvider.php (1)
addTemplateParameter(137-144)
🔇 Additional comments (5)
i18n/en.json (1)
13-15: LGTM! Using HTML comments as default values for customizable system messages.The approach of using HTML comment placeholders as default values is appropriate for messages intended to be customized by wiki administrators. The code in
ParameterProvider.phpcorrectly strips these comments when processing the messages.i18n/qqq.json (1)
13-15: LGTM! Clear documentation for translators.The instructions properly guide translators to preserve the HTML comment structure needed by the code's regex-based stripping logic.
src/SimpleBatchUpload.alias.php (1)
33-49: LGTM! Chinese locale aliases follow MediaWiki conventions.The aliases for zh-hans, zh-hant, and zh-hk are correctly structured and use appropriate translations for "BatchUpload" in each Chinese variant.
src/ParameterProvider.php (1)
63-64: Verify the regex handles edge cases correctly.The regex
/^[\s\n]*<!--[\s\S]*?-->[\s\n]*/uses a non-greedy match (*?) which correctly stops at the first-->. However, if the message contains multiple HTML comments, only the first one is stripped.Per the qqq.json documentation ("Do not add anything outside the first comment"), this behavior is intentional. Just confirm this aligns with expected usage.
src/UploadButtonRenderer.php (1)
128-133: Behavioral change: Template parameters now added unconditionally.The loop is now outside the
if ($templateName !== '')block, meaning whentemplateNameis empty, the empty string at$args[0]is also added as a template parameter. This appears intentional to support unnamed parser function calls like{{#batchupload:|arg1|arg2}}.However, when
templateNameis empty:
$args[0](the empty string) is not shifted off- The empty string gets added as a parameter via
addTemplateParameter('')- This produces
templateParameters = '|'initiallyThe normalization in
ParameterProvider::getUploadPageText()(line 55:preg_replace('/^\|+/', '|', ...)) handles the resulting||arg1|arg2by collapsing leading pipes. Confirm this is the intended behavior for backward compatibility.
| public function getUploadPageText(): string { | ||
|
|
||
| if ( $this->templateName === '' ) { | ||
| return ''; | ||
| $msgKey = 'simplebatchupload-filesummary'; | ||
| $templateName = $this->getParameter( self::IDX_TEMPLATENAME ); | ||
| $templateParams = preg_replace( '/^\|+/', '|', $this->getParameter( self::IDX_TEMPLATEPARAMETERS ) ); | ||
|
|
||
| if ( $this->templateName !== '' ) { | ||
| $msgKey = $msgKey . '-' . $templateName; | ||
| } | ||
|
|
||
| $fileSummaryMsg = Message::newFromKey( $msgKey, $templateParams ); | ||
|
|
||
| if ( $fileSummaryMsg->exists() ) { | ||
| return preg_replace( '/^[\s\n]*<!--[\s\S]*?-->[\s\n]*/', '', $fileSummaryMsg->plain() ); | ||
| } | ||
| else { | ||
| return '{{' . $templateName . $templateParams . '}}'; | ||
| } |
There was a problem hiding this comment.
Potential malformed wikitext when templateName is empty and message doesn't exist.
When $this->templateName is empty and the simplebatchupload-filesummary message doesn't exist (or is disabled), the fallback on line 67 returns '{{' . $templateName . $templateParams . '}}'. Since $templateName would be empty and $templateParams could be |param1|param2, this produces malformed wikitext like {{|param1|param2}}.
Consider returning an empty string or just the parameters without the template wrapper when $templateName is empty:
Proposed fix
if ( $fileSummaryMsg->exists() ) {
return preg_replace( '/^[\s\n]*<!--[\s\S]*?-->[\s\n]*/', '', $fileSummaryMsg->plain() );
}
else {
+ if ( $templateName === '' ) {
+ return '';
+ }
return '{{' . $templateName . $templateParams . '}}';
}1 participant
Currently, the default upload text for new files is very simple, they simply just add a template using subpage name, and using additional parameters when call through parser function.
On some wikis, uploaded files need to be tagged with copyright information. which is mandatory or recommended. Currently, SimpleBatchUpload do not have the ability to set a default upload text on root page, it means editors needs to write the upload text manually, and there is usually someone forget it, increases workload for maintenance on forementioned wikis.
This PR allows administrators set default texts for root page and subpages, at
MediaWiki:Simplebatchupload-filesummaryandMediaWiki:Simplebatchupload-filesummary-<subpagename>respectively, the message for the root page is also registered in the message file. Additionally, optional template parameters passed through parser functions can be substituted on$1in these system messages (prefix|included).Some other changes are also made in this PR:
simplebatchupload-parameterssystem message in the message file, a message for an existing feature.simplebatchupload-parameterssystem message, prettier in format, like the format inMediaWiki:Gadgets-definition.{{#batchupload:|arg1|arg2}}).zh-hans,zh-hant,zh-hk).This PR should be compatible with existing usages.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.