Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --with-translations option to create:plugin command #36

Closed
wants to merge 4 commits into from

Conversation

RomainMazB
Copy link
Contributor

Discussed with @bennothommo here: wintercms/winter#201

@github-actions
Copy link

github-actions bot commented Aug 9, 2021

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

RomainMazB added a commit to RomainMazB/docs-1 that referenced this pull request Oct 6, 2021
@RomainMazB
Copy link
Contributor Author

RomainMazB commented Oct 6, 2021

@maintainers Any feedback on this one? I've personally tested it and love it :)

The generated Plugin.php file includes the basic keys for the plugin's details, permissions and navigation example in the default app locale lang file.

Plugin.php example file generated with the command create:plugin Winter.SSOClient with this modification:

class Plugin extends PluginBase
{
    public function pluginDetails()
    {
        return [
            'name'        => 'winter.ssoclient::lang.plugin.name',
            'description' => 'winter.ssoclient::lang.plugin.description',
            'author'      => 'Winter',
            'icon'        => 'icon-leaf'
        ];
    }

    /**
     * ...
     */

    public function registerPermissions()
    {
        return []; // Remove this line to activate

        return [
            'winter.ssoclient.some_permission' => [
                'tab' => 'winter.ssoclient::lang.permissions.tab',
                'label' => 'winter.ssoclient::lang.permissions.label'
            ],
        ];
    }

    public function registerNavigation()
    {
        return []; // Remove this line to activate

        return [
            'ssoclient' => [
                'label'       => 'winter.ssoclient::lang.navigation.label',
                'url'         => Backend::url('winter/ssoclient/mycontroller'),
                'icon'        => 'icon-leaf',
                'permissions' => ['winter.ssoclient.*'],
                'order'       => 500,
            ],
        ];
    }
}

And the associated lang file:

<?php

return [
    'plugin' => [
        'name' => 'SSOClient',
        'description' => 'No description provided yet...'
    ],

    'permissions' => [
        'tab' => 'SSOClient',
        'label' => 'Some permission'
    ],

    'navigation' => [
        'label' => 'SSOClient'
    ]
];

Re-reading myself: maybe I shouldn't use the app locale config here due to the fact we are generating the file using english translations values. We should probably stick with the lang/en/lang.php file as english would probably be included into the final plugin's lang translations.
https://github.com/wintercms/storm/pull/36/files#diff-19d8c400587d85aa95b1b93249ba4edf1c017d9b0c185875aec671f32ee6bf06R34-R37

@LukeTowers
Copy link
Member

@bennothommo I've been toying around with just including translation support as the default for all of the scaffolds. I know it makes it more difficult for beginners, but it's a tough call between making it easier for beginners one time at the beginning of their usage of Winter CMS vs making it easier for developers all the time during their usage of Winter CMS. Any further thoughts on the subject?

@bennothommo
Copy link
Member

bennothommo commented Oct 7, 2021

@LukeTowers I would say I'm leaning towards not, simply because it's not always a requirement - there'd be people (like me) who make custom plugins all the time without any intention of releasing them, and language support means double-handling of strings (granted, translation is super simple in Winter).

On the other hand, the Builder plugin pretty much forces it on plugins created from there anyway, and the Builder plugin is specifically targeting beginners.

So, on balance, I'd say yes to including it by default, but I'd personally like it if it had an option to exclude it.

@RomainMazB
Copy link
Contributor Author

RomainMazB commented Oct 7, 2021

I wanted to add my point of view and an introduce an idea which could be a third alternative (bad or not, I'll let you decide).

Scaffolding commands are great

In both Laravel and Winter, scaffolding commands is one of the greatest feature: it decrease the time to develop an app and increase the way to build it by providing a skeleton of how you should build your controller, model and so on.

The starting point of all my scaffolding commands improvement is an app where I decided to build many plugins that I wanted to share on the marketplace the already-available plugins didn't fit my need or was poor-quality plugins.
This experience made me repeat myself over and over on each plugin I built for this app. That made me realize which feature was missing and what a developer needed to publish a high-quality plugin.
All this talk to just say "We should spend some time on scaffolding commands to power-up their features as this is one of the first thing a developer will look into when it start using WinterCMS"

Decide only once

The idea I wanted to introduce, is a config file:

  • @LukeTowers is right saying that for a developer who share most of their plugins and so want them to be localized, --with-translations option is a thing the developer don't want to think about on each command.
  • @bennothommo is right saying that the beginners probably don't want to think about localization because their first plugins are probably just for themselves or a to-do list to just learn or discover the CMS to decide if it fits their need.
    Plus - as mentioned by Ben -, sometimes you just don't want to share the plugin so you don't want to spend your time on this stuff

What if we create a config/scaffolding.php file? This file would look like this at the beginning:

<?php

return [

    /*
    |--------------------------------------------------------------------------
    | Localization scaffoldings
    |--------------------------------------------------------------------------
    |
    | When this option is set to true, all the scaffolding commands will
    | contain localization keys, and basic lang files will be created
    | at the starting point of the plugin creation.
    |
    */

    'with-translation' => false,

    /*
    |--------------------------------------------------------------------------
    | Scaffolding languages
    |--------------------------------------------------------------------------
    |
    | When create:plugin runs, all the lang files for the languages in this array
    | will be created with the corresponding key in each lang file.
    |
    | Only the english lang file will contain the generated values, it's up to you to fill the others
    |
    */

    'languages' => ['en'],
];

The GeneratorCommand class could be improved :

  • to handle this new configs and ease the way its child-classes choose between the needed stubs
  • to introduce a way to disable/enable the option momentarily like: php artisan create:plugin --with-translations=no

Going deeper

1./ In plugins localization

To go deeper in the ease for plugins localization - and as already discussed with @mjauvin -, we could introduce a command which could compare the lang files to fill the missing keys in all the lang files, the main lang file could be a config into this new scaffolding config subset but no key would be erased from others (may be we could add a comment on those lines like // Key not found in /lang/en/lang.php)
This command should be usable on the core files and a starting point has been made by mjauvin here

2./ In the ease for experienced developer experience

When you build plugins, you mostly use the same Author name, so why not store this into the new scaffolding config file?

<?php

return [

    /*
    |--------------------------------------------------------------------------
    | Author Name
    |--------------------------------------------------------------------------
    |
    | Fill into this config the default author name for all the plugins scaffolding commands
    | When not an empty string, you will not need to put your AuthorName anymore.
    |
    | Eg: php artisan create:plugin MyPlugin
    |
    */

    'author-name' => '',

We still should be able to pass a full plugin code if you need to use scaffolding command into another plugin namespace.

Conclusion

Easing the way to build - international or not - plugins will improve the marketplace plugins quality, and scaffolding commands are the best way to do that and that's why I opened the forum discussion about the features I felt missing. But we must not complicate the CMS discovering for the newcomers who may build the next top-level plugin on the marketplace.

We may not should choose between the beginner or the confirmed developer but introduce a new way to ease both developers experience.

@bennothommo
Copy link
Member

@RomainMazB good thoughts.

The only issue we'll encounter with a config file is that it's generally difficult to deliver out to users as an update - once Winter is installed, we generally don't touch the config directory, and Composer will only affect the vendor, modules, plugins and themes directories. Our precedent so far is to only introduce config files in major releases.

This would mean that the option of including translations or not would still need to be controllable via the scaffold command.

To make consideration on all these points simpler - I think we should operate under the assumption that it's mainly experienced developers using the scaffold commands, and encourage all beginner developers to use the Builder plugin. My ultimate goal with the Builder plugin is to be used to the full extent of editing all facets of a plugin - I think the original plan of it being just a bootstrap for a new plugin, while noble, results in most plugin development being a two-step process - using the Builder to start, then having to use your code editor to actually get stuff done.

@RomainMazB
Copy link
Contributor Author

The only issue we'll encounter with a config file is that it's generally difficult to deliver out to users as an update - once Winter is installed, we generally don't touch the config directory

Is that a "winter-personal guidelines" or a composer limitation? I never used it but composer looks like it can run a post-update-cmd if we provide one into the composer.json file. As I see in the composer documentation, we could just store a config file stub into the release of Winter and just run a cp command to copy it into the config directory. Am I wrong or missing something?

@bennothommo
Copy link
Member

@RomainMazB it's more a "Winter" rule, I suppose. The same issue would crop up if we wanted to push out an update to the composer.json file to facilitate this change - only way we could do it is to ask people to manually update their composer.json files on already-installed copies of Winter.

Theoretically, we could introduce an update "handler" in Winter that would run once the first time Winter CMS is used, after an update occurred, but I think that's rather hacky.

@RomainMazB
Copy link
Contributor Author

RomainMazB commented Oct 7, 2021

I don't want to push the Winter's lines but not being able to introduce a config file is a limitation I feel unfair. As a a parallel-example, laravel/laravel is publishing its assets in post-update-cmd since this summer

Meaning this would be the case into L9: we may introduce a similar process into the next major release of Winter (and all my config file stuff after that).

@bennothommo
Copy link
Member

@RomainMazB that's what I'm saying though. We can certainly introduce this in a major release, but if you wanted to include it right now as a part of this PR, then it won't work.

@skripteria
Copy link

skripteria commented Oct 7, 2021

Interesting discussion, since I am actually both sort of a Winter beginner but also a plugin developer for my actual project here is how I handle it at the moment.

My project only supports 2 languages (de and en), and for this scenario I decided to add additional translation fields to my model (I do need to support external data feeds too, so much easier here).
I started the first plugin with builder but soon realized that it was easier just to use the scaffoling commands and the speed you can achieve with this is pretty amazing.

I also realized that plugins are also a great way to keep control fo the content within bigger companies with multiple unexperienced editors. So to protect the overall design of the pages I keep most of the content in my plugins. So yes, these are build just for this specific use case and I have no intention of publishing it.

The perfect solution for me would a scaffolding command with an additional parameter like -l to add language support:

php artisan create:plugin -l

P.S.: forgot to mention the my plugin interfaces dont need language support, just the content.

@RomainMazB
Copy link
Contributor Author

RomainMazB commented Oct 7, 2021

@bennothommo yeah, that's what you're saying, with the difference that after that (if we introduce a post-update-cmd into 1.2.0), we will be able to create new config files into a minor release, the "Winter rule" will change.
And if the idea of this scaffolding config file seems fine to everyone, we still are able to put the config calls with default values (synced to the actual behavior) as the second parameter, and those who want to could create this config file manually without the need to wait for the next major release.

@BWurm, thanks for the feedback, your are confirming some feelings most of us have: the builder is made for beginners, but then beginners quickly understand that they could do faster and without any limitation with the scaffolding commands. Builder is still an awesome plugin which quickly introduce the newcomers to the "woaw, this CMS is awesome" we all know.

I'll wait until everyone can give their opinion before going further on this PR.

@LukeTowers
Copy link
Member

Given that a strong selling point of Winter CMS is the ease of which it can provide a multilingual experience especially within the international market I think we should definitely be pushing for having localization as the default experience like we are already with the builder plugin. I'm not so sure about the idea of having config file for these items, beside the issues mentioned by @bennothommo it just becomes one more thing to have to configure and somewhat slows down the experience of scaffolding IMO.

I think we should include it by default on all applicable scaffolding commands but provide the option to generate non-localized versions as is being done now through something like --exclude-translations

@damsfx
Copy link
Contributor

damsfx commented Oct 8, 2021

I'm joining @LukeTowers .
I think providing a multilingual user experience is necessary but having the option to revert back to the default mode (without translation) would be more beneficial than having to go through a config file again.
I almost never touch them, except to override the configuration of a plugin that allows it, I prefer the environment file.

For the best user experience, I think that translations should also be the rule in controllers (#35).
There is nothing more annoying than having to go over all the view files again, to include translations; because most of the time, plugins are made for customers who want an interface in their native language.

@LukeTowers LukeTowers added this to the v1.1.8 milestone Nov 13, 2021
@github-actions
Copy link

This pull request will be closed and archived in 3 days, as there has been no activity in the last 60 days.
If this is still being worked on, please respond and we will re-open this pull request.
If this pull request is critical to your business, consider joining the Premium Support Program where a Service Level Agreement is offered.

@LukeTowers
Copy link
Member

Replaced by wintercms/winter#486

@LukeTowers LukeTowers closed this Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants