Add custom kml filename support#847
Merged
JeroenDeDauw merged 2 commits intoProfessionalWiki:masterfrom Apr 2, 2026
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new exported Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/SemanticMW/KmlPrinter.php`:
- Around line 61-63: The filename presence check is inconsistent; replace the
`!empty` predicate with the same explicit check used elsewhere so both branches
use the same logic (e.g. require isset($this->params['filename']) and compare
!== ''), and then call $link->setParameter($this->params['filename'],
'filename') only when that check passes; update the block that currently uses
`!empty($this->params['filename'])` so it matches the other branch's check to
avoid differing behavior for values like '0'.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f78c0f0-79cb-43aa-b2bf-20ba25b2c5ee
📒 Files selected for processing (2)
i18n/en.jsonsrc/SemanticMW/KmlPrinter.php
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/SemanticMW/KmlPrinter.php (1)
111-114: Avoid duplicating the default filename literal.
'kml.kml'is defined here and again ingetFileName(). A shared class constant reduces drift risk.♻️ Proposed refactor
class KmlPrinter extends FileExportPrinter { + private const DEFAULT_KML_FILENAME = 'kml.kml'; @@ $definitions['filename'] = [ 'message' => 'semanticmaps-kml-filename', - 'default' => 'kml.kml', + 'default' => self::DEFAULT_KML_FILENAME, ]; @@ public function getFileName( QueryResult $queryResult ) { if ( $this->params['filename'] !== '' ) { return $this->params['filename']; } - return 'kml.kml'; + return self::DEFAULT_KML_FILENAME; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SemanticMW/KmlPrinter.php` around lines 111 - 114, Replace the duplicated literal 'kml.kml' with a single class constant to avoid drift: add a constant like DEFAULT_FILENAME = 'kml.kml' to the KmlPrinter class, then use that constant in the $definitions['filename'] default and in getFileName() (replace occurrences of 'kml.kml' with self::DEFAULT_FILENAME) so both places reference the same identifier.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/SemanticMW/KmlPrinter.php`:
- Around line 111-114: Replace the duplicated literal 'kml.kml' with a single
class constant to avoid drift: add a constant like DEFAULT_FILENAME = 'kml.kml'
to the KmlPrinter class, then use that constant in the $definitions['filename']
default and in getFileName() (replace occurrences of 'kml.kml' with
self::DEFAULT_FILENAME) so both places reference the same identifier.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5434f679-5f4a-4106-baa3-f4df6c0a3054
📒 Files selected for processing (2)
i18n/en.jsonsrc/SemanticMW/KmlPrinter.php
✅ Files skipped from review due to trivial changes (1)
- i18n/en.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/SemanticMW/KmlPrinter.php (1)
111-114: Consider centralizing the default filename constant.Line 113 introduces
'kml.kml', but the same default is also hardcoded ingetFileName. A shared class constant would avoid future drift.Refactor sketch
class KmlPrinter extends FileExportPrinter { + private const DEFAULT_KML_FILENAME = 'kml.kml'; @@ $definitions['filename'] = [ 'message' => 'semanticmaps-kml-filename', - 'default' => 'kml.kml', + 'default' => self::DEFAULT_KML_FILENAME, ]; @@ - return 'kml.kml'; + return self::DEFAULT_KML_FILENAME; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SemanticMW/KmlPrinter.php` around lines 111 - 114, Replace the two hardcoded defaults by introducing a single class-level constant (e.g. DEFAULT_FILENAME) and use it wherever the default filename is needed: update the $definitions['filename'] default array entry (the assignment that sets 'default' => 'kml.kml') to reference the new constant, and update the getFileName method to return that same constant when no filename is provided; ensure the constant is defined in the KmlPrinter class and used in both places to prevent drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/SemanticMW/KmlPrinter.php`:
- Around line 111-114: Replace the two hardcoded defaults by introducing a
single class-level constant (e.g. DEFAULT_FILENAME) and use it wherever the
default filename is needed: update the $definitions['filename'] default array
entry (the assignment that sets 'default' => 'kml.kml') to reference the new
constant, and update the getFileName method to return that same constant when no
filename is provided; ensure the constant is defined in the KmlPrinter class and
used in both places to prevent drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 141ab2cd-0110-46a1-80c1-7ac8accd642a
📒 Files selected for processing (2)
i18n/en.jsonsrc/SemanticMW/KmlPrinter.php
✅ Files skipped from review due to trivial changes (1)
- i18n/en.json
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/SemanticMW/KmlPrinter.php (1)
111-114: Avoid duplicating the default filename literal.
'kml.kml'is now defined here and again in the fallback return path. Consider a class constant to keep this single-sourced.Proposed refactor
class KmlPrinter extends FileExportPrinter { + private const DEFAULT_KML_FILENAME = 'kml.kml'; @@ $definitions['filename'] = [ 'message' => 'semanticmaps-kml-filename', - 'default' => 'kml.kml', + 'default' => self::DEFAULT_KML_FILENAME, ]; @@ public function getFileName( QueryResult $queryResult ) { if ( $this->params['filename'] !== '' ) { return $this->params['filename']; } - return 'kml.kml'; + return self::DEFAULT_KML_FILENAME; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/SemanticMW/KmlPrinter.php` around lines 111 - 114, The literal 'kml.kml' is duplicated in the file; introduce a class constant (e.g. DEFAULT_FILENAME) in the KmlPrinter class and replace both occurrences: set $definitions['filename']['default'] to self::DEFAULT_FILENAME and use self::DEFAULT_FILENAME in the fallback return path (and any other places referencing the literal) so the default filename is single-sourced and maintainable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/SemanticMW/KmlPrinter.php`:
- Around line 111-114: The literal 'kml.kml' is duplicated in the file;
introduce a class constant (e.g. DEFAULT_FILENAME) in the KmlPrinter class and
replace both occurrences: set $definitions['filename']['default'] to
self::DEFAULT_FILENAME and use self::DEFAULT_FILENAME in the fallback return
path (and any other places referencing the literal) so the default filename is
single-sourced and maintainable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cc7ea9ae-87a5-4052-be29-c3d14a6db1e6
📒 Files selected for processing (2)
i18n/en.jsonsrc/SemanticMW/KmlPrinter.php
✅ Files skipped from review due to trivial changes (1)
- i18n/en.json
- Change default from 'kml.kml' to '' so the filename parameter only affects link URLs and Content-Disposition when explicitly set, preserving existing behavior for all current users - Enforce .kml extension on custom filenames to prevent misleading download names (e.g. 'data.html' → 'data.html.kml') - Add missing qqq.json message documentation for translators Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently the filename of the outputted kml file is hard coded to
kml.kml.This change makes it possible to provide a custom filename.
Still defaults to
kml.kml.Summary by CodeRabbit
New Features
Localization