Skip to content

i18n(plugin-builder): replace hardcoded SQL-table prompt with EPL_ADL…#5622

Open
Kanonimpresor wants to merge 4 commits into
e107inc:masterfrom
Kanonimpresor:fix/pluginbuilder-i18n-step2-table
Open

i18n(plugin-builder): replace hardcoded SQL-table prompt with EPL_ADL…#5622
Kanonimpresor wants to merge 4 commits into
e107inc:masterfrom
Kanonimpresor:fix/pluginbuilder-i18n-step2-table

Conversation

@Kanonimpresor
Copy link
Copy Markdown
Contributor

@Kanonimpresor Kanonimpresor commented May 9, 2026

…AN_258

The Plugin Builder step 2 had a hardcoded English string telling the user to select a SQL table. Move it into lan_plugin.php as EPL_ADLAN_258 so it can be translated, and inject the plugin's _sql.php filename via the standard [x] placeholder.

Motivation and Context

Description

How Has This Been Tested?

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist

`lan_print.php` defined `PAGE_NAME => "Printer Friendly"` at the top of
the language pack. `e107_handlers/emailprint_class.php` includes
`lan_print.php` whenever it loads (e.g. from a `{PRINTICON}` shortcode
on a news widget rendered before the page title), so any unrelated page
that hadn't already defined `PAGE_NAME` ended up with "Printer Friendly"
showing in the `<title>` tag, falling back through
`header_default.php`'s renderTitle().

Drop the `PAGE_NAME` key from the language pack and rename it
`LAN_PRINT_PAGE_NAME`. `print.php` now sets its own page title with
`e107::title(LAN_PRINT_PAGE_NAME)` right after `e107::coreLan('print')`,
so the print endpoint itself still reads "Printer Friendly | <SITENAME>"
while the side-effect leakage on other pages goes away.

A unit test (`lanPrintTest`) asserts the language pack does not carry a
`PAGE_NAME` entry, guarding against regression.

Closes e107inc#5606
Copy link
Copy Markdown
Contributor

@e107help e107help Bot left a comment

Choose a reason for hiding this comment

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

Boas @Kanonimpresor! The instinct to lift those hardcoded strings into lan_plugin.php is the right one, and the wizard's been overdue for it. A few patterns I'd ask you to undo before this lands, though.

I haven't run this against a live site, just traced the code.

1. The defined('EPL_ADLAN_XXX') ? EPL_ADLAN_XXX : "English fallback" ternaries don't do anything

plugin_class.php loads its own LAN at file-include time:

That's e107::coreLan('plugin', true), which runs before the class body is even parsed. By the time getAddonsDiz() or addons() is called, every EPL_ADLAN_* constant in lan_plugin.php is defined. The fallback branch is unreachable.

Worse, the fallback duplicates the canonical English string, so there are two sources of truth in two files. The next person who tweaks the LAN string and forgets to update the fallback in plugin_class.php will drift them silently. That's the opposite of what i18n is supposed to give us.

The rest of the file uses these constants bare. Forty-six direct references in plugin_class.php and zero wrapped in defined(). Please use the bare constant:

'_blank' => EPL_ADLAN_259,

Same goes for the lazy-init in getAddonsDiz(). It's only needed if the constants aren't defined when the property initializer runs. They are. Once the defensive ternaries come off, you can keep $plugin_addons_diz initialized inline as a class property the way it was, and the getter goes back to a one-liner. The diff shrinks a lot.

The commit body on 8e0db14 says the guards "preserve back-compat for third-party translators". I'd take that as the AI guessing rather than reading the codebase. :-? Translators don't need that guarantee; the LAN loader already covers them.

2. Inconsistent treatment within the PR

EPL_ADLAN_258 (commit b2d7d22) is a bare constant in the str_replace call. EPL_ADLAN_259..289 (commits 8e0db14, 2c7553b) are all wrapped in the ternary. Two patterns for the same kind of code in the same PR is a tell. Pick the bare one and apply it everywhere.

3. Inline maintainer notes got dropped

The original property carried trailing comments next to several entries:

  • 'e_dashboard' => "...", // Admin Front-Page addon.
  • 'e_header' => "...", // loaded in header prior to javascript manager.
  • 'e_footer' => "...", // Loaded in footer prior to javascript manager.
  • 'e_url' => "...", // simple mod-rewrite.
  • 'e_sitelink' => "...", // sitelinks generator.

git blame puts those on @CaMer0n in 2016. They're internal hints for the next reader, not user-visible copy, and moving the user strings to LAN doesn't require dropping the notes. Please keep them next to the array entries.

4. Indentation drift in lan_plugin.php

The file uses 4-space indent throughout. EPL_ADLAN_258 is 4-space; EPL_ADLAN_259..289 are tab-indented. Worth normalising back to 4 spaces so the diff stays uniform with the rest of the file.

5. There's a lanVars() helper for the [x] placeholder

e107::getParser()->lanVars($lan, $vals, $bold) (defined in e107_handlers/e_parse_class.php:3978, used 30-plus places) is the established way to substitute [x]/[y]/[z] placeholders in LAN strings. Equivalent for step 2's prompt:

$lanSelectTable = $tp->lanVars(EPL_ADLAN_258, $this->pluginName . '_sql.php', true);

The hand-rolled str_replace works, but the helper is what the rest of the codebase uses for this exact pattern.

6. Version metadata inside a user-facing string

EPL_ADLAN_289 is:

Customize the [Print] function for your plugin's content (added in v2.3.1).

That parenthetical will look odd to anyone reading the wizard a few years from now, and translators have to translate it. Code comment if it matters; not LAN string.

(For posterity: e_print landed in 07faad666 by @CaMer0n against issues #2726 / #4452, released in v2.2.0, not v2.3.1.)

7. Commit shape

The three commits are slicing the work along reasonable seams, but the framing across them is inconsistent (b2d7d22 uses bare constants; 8e0db14 introduces the defensive pattern and the lazy-init; 2c7553b adds a real feature ride-along inside an "i18n" PR). If you're up for restructuring the history:

  • Commit A — pure i18n of the wizard. EPL_ADLAN_258..288, bare constants, no lazy-init, no defensive ternaries, inline // notes preserved.
  • Commit B — add the missing e_print description. New array entry plus EPL_ADLAN_289. This one isn't i18n; it's a Plugin Builder bug fix that's been hiding inside the i18n diff.

Squashing all three together would also be fine, but it'd hide the e_print change inside an i18n diff and make git log --oneline less useful later.

A failing test you could drop in

Touching getAddonsDiz() is the kind of thing one small Codeception fixture would lock in. Something like e107_tests/tests/unit/PluginAddonsDizTest.php:

<?php
class PluginAddonsDizTest extends \Codeception\Test\Unit
{
    public function testEPrintAddonHasDescription()
    {
        require_once e_HANDLER . 'plugin_class.php';
        $plg = new e107plugin();
        $this->assertNotEmpty(
            $plg->getAddonsDiz('e_print'),
            "e_print should have a description in getAddonsDiz()"
        );
    }
}

That fails on master today (the empty cell next to e_print in the wizard) and passes once your commit B lands. Feel free to drop it in alongside; if it doesn't fit your scope, leave it and we'll land it as a follow-up.


That's the lot from me on this pass. Intent's solid and lifting these into lan_plugin.php is the right direction. Once the defensive scaffolding comes off and Cameron's notes go back in, it'll read like the small targeted PR it wants to be. 8-)

Cheers, and shout if anything above doesn't land.

…L_ADLAN_258-288)

- Replace inline English strings in step 2 (SQL table prompt) and step 3 (addon descriptions) with language constants.
- Refactor e107plugin::$plugin_addons_diz: convert the hardcoded English array property into a lazy-initialized map inside getAddonsDiz() that resolves EPL_ADLAN_264-288 at runtime, so addon descriptions follow the user's language.
- Use e107::getParser()->lanVars() for [x] placeholder substitution (matches the rest of the file).
- Maintainer comments preserved inline next to each EPL_ADLAN_* mapping.
The e_print addon was listed in e107plugin::$_addon_types since v2.3.1 but had no entry in the description map used by the Plugin Builder wizard, so its tooltip rendered empty.

- Add 'e_print' => EPL_ADLAN_289 mapping inside getAddonsDiz().
- Add the EPL_ADLAN_289 language string.
@Kanonimpresor Kanonimpresor force-pushed the fix/pluginbuilder-i18n-step2-table branch from 2c7553b to 03495c1 Compare May 17, 2026 10:02
…ription

Regression guard for the e_print addon (added in v2.3.1 to $_addon_types) which was missing from the description map used by the Plugin Builder wizard, causing an empty tooltip in step 3.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants