Make default output filename suffix configurable (fixes #259)#429
Merged
kwart merged 4 commits intoJul 5, 2026
Merged
Conversation
…t#259) Add an output.suffix advanced-config key (default "_signed") read via AppConfig.defaultOutSuffix(), so the suffix appended to derive the output filename can be localized. Replaces the hardcoded "_signed" in the core getOutFileX(), the JavaFX and Swing GUIs, and the CLI -osuffix default.
There was a problem hiding this comment.
Pull request overview
Adds a new output.suffix advanced-config key (default _signed) and wires it through the shared signing model, both GUIs, and the CLI so the default output filename marker can be localized without requiring a per-invocation flag.
Changes:
- Introduce
AppConfig.defaultOutSuffix()backed byadvanced.properties/ bundled defaults, and use it when deriving default output filenames. - Update CLI
--helptext and documentation to describe the newoutput.suffixknob. - Add test coverage around the new accessor.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/JSignPdf.adoc | Documents output.suffix for GUI behavior and CLI option defaults. |
| jsignpdf/src/test/java/net/sf/jsignpdf/utils/AppConfigTest.java | Adds assertions for the new defaultOutSuffix() accessor. |
| jsignpdf/src/main/java/net/sf/jsignpdf/SignPdfForm.java | Uses AppConfig.defaultOutSuffix() for Swing GUI output filename suggestion. |
| jsignpdf/src/main/java/net/sf/jsignpdf/SignerOptionsFromCmdLine.java | Uses configured suffix as the CLI default and injects it into help text formatting. |
| jsignpdf/src/main/java/net/sf/jsignpdf/fx/view/MainWindowController.java | Uses AppConfig.defaultOutSuffix() for JavaFX output filename suggestion/default. |
| engines/api/src/main/resources/net/sf/jsignpdf/translations/messages.properties | Updates hlp.outSuffix help text to include a formatted default suffix. |
| engines/api/src/main/resources/net/sf/jsignpdf/conf/advanced.default.properties | Adds bundled default output.suffix=_signed with documentation. |
| engines/api/src/main/java/net/sf/jsignpdf/utils/AppConfig.java | Adds defaultOutSuffix() accessor. |
| engines/api/src/main/java/net/sf/jsignpdf/BasicSignerOptions.java | Uses configured suffix when computing getOutFileX() default output filename. |
| distribution/doc/release-notes/3.1.0.md | Notes the new configurable default output suffix feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+29
to
38
| @Test | ||
| public void defaultOutSuffixFallsBackToConstants() { | ||
| // DEFAULT_OUT_SUFFIX is the literal fallback when the bundled defaults resource is missing or empty. | ||
| String suffix = AppConfig.defaultOutSuffix(); | ||
| assertEquals("Default value should match Constants", | ||
| net.sf.jsignpdf.Constants.DEFAULT_OUT_SUFFIX, | ||
| // If a developer has a real override, this assertion can be skipped — but on a clean checkout the | ||
| // bundled default matches the Constants literal. | ||
| suffix.isEmpty() ? net.sf.jsignpdf.Constants.DEFAULT_OUT_SUFFIX : suffix); | ||
| } |
| hlp.outPath=folder in which the signed documents will be stored. Default value is current folder. | ||
| hlp.outPrefix=prefix for signed file. Default value is empty prefix. | ||
| hlp.outSuffix=suffix for signed filename. Default value is "_signed". (e.g. sign process on file mydocument.pdf will create new file mydocument_signed.pdf) | ||
| hlp.outSuffix=suffix for signed filename. Default value is "{0}" (configurable with the output.suffix key in advanced.properties). E.g. sign process on file mydocument.pdf will create new file mydocument{0}.pdf |
- Use {0} placeholder in hlp.outSuffix across all locale files so non-English
--help reflects the configured output.suffix; tidy the French string.
- Document the MessageFormat apostrophe-doubling requirement for translators.
- Re-resolve AppConfig.defaultOutSuffix() in loadCmdLine when -osuffix is
absent, instead of trusting the construction-time field initializer.
- Replace the tautological AppConfigTest with one that exercises the
output.suffix key and the bundled-default fallback.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Now that master's advanced-override mechanism is merged in, lock in finding intoolswetrust#3: loadCmdLine must re-resolve the configured output.suffix when -os is absent (verified to fail without the else branch), and an explicit --out-suffix still wins. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Member
|
Thanks for this @AymenJeddou! I reviewed it and pushed a few follow-up commits to the branch:
Merged latest |
Contributor
Author
|
Thanks @kwart for the thorough review and for taking the time to push the fixes directly, that's helpful. The localization gap makes sense, I missed that the {0} placeholder only landed in the English file, which defeats the point of the feature for exactly the users it targets. The translator note on apostrophe doubling is a good addition too. The field-initializer ordering issue is a good catch. I hadn't accounted for the override mechanism from #435 applying after construction, so re-resolving in loadCmdLine is the correct place for it. The regression tests covering both the absent -os case and the explicit override winning look solid. Everything reads well to me, nothing I'd want changed. Happy to have this go in as is. |
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
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.
Fixes #259
The default "_signed" suffix used to derive the output filename was hardcoded.
This adds an
output.suffixkey inadvanced.properties(bundled default_signed), exposed viaAppConfig.defaultOutSuffix(), so non-English userscan localize it (e.g.
_firmado).Applied consistently across:
BasicSignerOptions.getOutFileX()(used by the signing engines)MainWindowController)SignPdfForm)-osuffix(the--helptext now shows the configured value)Docs updated:
JSignPdf.adoc,advanced.default.properties, 3.1.0 releasenotes. Added
AppConfigTestcoverage.The existing
-osuffixflag still overrides per-invocation. No Preferencesdialog control was added (file-editable only) — happy to add one if preferred.