NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[FrameworkBundle] Add completion to debug:translation command#43644

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:5.4fromalexandre-daubois:feat/debug-translation-autocomplete

Oct 29, 2021

Merged

[FrameworkBundle] Add completion to debug:translation command#43644

fabpot merged 1 commit intosymfony:5.4fromalexandre-daubois:feat/debug-translation-autocomplete

Oct 29, 2021

Conversation

Copy link

Member

alexandre-daubois commentedOct 22, 2021•
edited
Loading

Q	A
Branch?	5.4
Bug fix?	no
New feature?	yes
Deprecations?	no
Tickets	Part of#43594
License	MIT
Doc PR	NA

carsonbot added Status: Needs Review Console Feature labels

Oct 22, 2021

carsonbot added this to the5.4 milestone

Oct 22, 2021

alexandre-daubois force-pushed thefeat/debug-translation-autocomplete branch 3 times, most recently from2d1f985 tob0a55a4Compare

October 22, 2021 07:01

Copy link

Member

Choose a reason for hiding this comment

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

This is a very long list. Since this command describes the state of the application, I think it would be more useful if only the used locales were listed.

Copy link

MemberAuthor

Choose a reason for hiding this comment

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

No problem, taking a look at it!

Copy link

Contributor

stephenkhooOct 23, 2021•
edited
Loading

Choose a reason for hiding this comment

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

I'm working on another translation related command that require to autocomplete for locale too.
I'm thinking if we're going to have only used locales, maybe we will need to addgetUsedLocals() intoExtractorInterface andTranslationReaderInterface

So far I only know the we need to do as below if we're adding into the interface.
TranslationReader need to put the logic for getting the Locales
ChainExtractor andPhpExtractor also need to add the logic to get used Locales.

Quick global search within the repo also gave me one more class that needs update:
Symfony\Bridge\Twig\Translation\TwigExtractor

But not quite sure if it will affect other third party package.

Copy link

Contributor

stephenkhooOct 23, 2021

Choose a reason for hiding this comment

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

https://github.com/symfony/symfony/blob/4.4/src/Symfony/Component/Translation/Loader/LoaderInterface.php

Also might need to consider third party translation loader included as the autocomplete suggestions.

Copy link

Member

stofOct 23, 2021

Choose a reason for hiding this comment

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

FrameworkBundle has a concept of enabled locales. I suggest using those for the completion.

Copy link

MemberAuthor

alexandre-dauboisOct 24, 2021

Choose a reason for hiding this comment

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

Does this really make sense to limit locales completion to the "enabled" ones in the project, aside that it seems tricky to get these enabled locales? Symfony already bundles a lot of messages in dozens of locales. Restricting completion could kind of prevent displaying messages defined by Symfony in domains like thevalidators one.

Copy link

Member

fabpotOct 25, 2021•
edited
Loading

Choose a reason for hiding this comment

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

Here, it's only about completion, I'm fine with auto-completing only the enabled locales. If you want another one, just type it explicitly.

Copy link

MemberAuthor

alexandre-dauboisOct 25, 2021

Choose a reason for hiding this comment

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

Alright, I'm on it!

Copy link

Member

stofOct 25, 2021

Choose a reason for hiding this comment

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

@stephenkhoo to access the enabled locales, use DI, i.e. inject them in the command constructor.

Copy link

Contributor

stephenkhooOct 25, 2021

Choose a reason for hiding this comment

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

Got it, thanks

alexandre-daubois force-pushed thefeat/debug-translation-autocomplete branch fromb0a55a4 toa1c32b4Compare

October 22, 2021 07:28

Copy link

MemberAuthor

alexandre-daubois commentedOct 22, 2021

Completion don't have to be exhaustive. Bundle names can be completed getting the name of each bundle provided by$kernel->getBundles(). That would be useful.

Suits me fine. Added!

alexandre-daubois force-pushed thefeat/debug-translation-autocomplete branch froma1c32b4 to1388391Compare

October 22, 2021 07:29

wouterj mentioned this pull request

Oct 22, 2021

[DX] Add completion to Symfony commands#43594

Closed

54 tasks

stephenkhoo reviewed

Oct 23, 2021

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Command/TranslationDebugCommand.phpShow resolvedHide resolved

stephenkhoo mentioned this pull request

Oct 23, 2021

[FrameworkBundle] Add completion feature on translation:update command#43676

Merged

10 tasks

derrabus added FrameworkBundle and removed Console labels

Oct 24, 2021

carsonbot changed the title~~[Console] Add completion to debug:translation command~~[FrameworkBundle] Add completion to debug:translation command

Oct 24, 2021

alexandre-daubois force-pushed thefeat/debug-translation-autocomplete branch 3 times, most recently from1529f97 to454d1b5Compare

October 25, 2021 16:32

fabpot requested changes

Oct 26, 2021

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Command/TranslationDebugCommand.phpShow resolvedHide resolved

carsonbot added Status: Needs Work and removed Status: Needs Review labels

Oct 26, 2021

alexandre-daubois force-pushed thefeat/debug-translation-autocomplete branch from454d1b5 to621d83cCompare

October 26, 2021 14:19

[Console] Add completion to debug:translation command

086a8b4

alexandre-daubois force-pushed thefeat/debug-translation-autocomplete branch from621d83c to086a8b4Compare

October 26, 2021 14:21

fabpot added a commit that referenced this pull request

Oct 29, 2021

feature#43676[FrameworkBundle] Add completion feature on translatio…

375a89a

…n:update command (stephenkhoo)This PR was squashed before being merged into the 5.4 branch.Discussion----------[FrameworkBundle] Add completion feature on translation:update command| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | Part of#43594| License       | MIT| Doc PR        | -Adding completion for translation:update.- [x] locale- [X] bundle- [X] --format- [X] --domain- [X] --sortTest for- [x] locale- [x] bundle- [X] --format- [X] --domain- [X] --sortLocale completion still under discussion in#43644 (review)Locale and bundle test still not completeCommits-------2f301ae [FrameworkBundle] Add completion feature on translation:update command

symfony-splitter pushed a commit to symfony/framework-bundle that referenced this pull request

Oct 29, 2021

feature #43676 [FrameworkBundle] Add completion feature on translatio…

a9c9a15

…n:update command (stephenkhoo)This PR was squashed before being merged into the 5.4 branch.Discussion----------[FrameworkBundle] Add completion feature on translation:update command| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Tickets       | Part of #43594| License       | MIT| Doc PR        | -Adding completion for translation:update.- [x] locale- [X] bundle- [X] --format- [X] --domain- [X] --sortTest for- [x] locale- [x] bundle- [X] --format- [X] --domain- [X] --sortLocale completion still under discussion insymfony/symfony#43644 (review)Locale and bundle test still not completeCommits-------2f301ae9e1 [FrameworkBundle] Add completion feature on translation:update command

fabpot approved these changes

Oct 29, 2021

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels

Oct 29, 2021

Copy link

Member

fabpot commentedOct 29, 2021

Thank you@alexandre-daubois.

fabpot mentioned this pull request

Oct 29, 2021

[FrameworkBundle] Add completion to debug:translation commandsymfony/symfony-docs#16037

Closed

fabpot merged commit12f8fae intosymfony:5.4

Oct 29, 2021

GromNaN reviewed

Nov 2, 2021

View reviewed changes

src/Symfony/Bundle/FrameworkBundle/Tests/Command/TranslationDebugCommandTest.php


		yield'bundle' => [
		['fr','--domain','messages',''],
		['ExtensionWithoutConfigTestBundle','extension_without_config_test'],

Copy link

Member

GromNaNNov 2, 2021

Choose a reason for hiding this comment

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

I get an exception when I use the bundle_name in snake case. Does it work for you? (It's ok with BundleName)

$bin/console debug:translation en framework  "framework/translations" is neither an enabled bundle nor a directory.

Copy link

MemberAuthor

alexandre-dauboisNov 3, 2021

Choose a reason for hiding this comment

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

Taking a look ASAP 👍

This was referencedNov 5, 2021

Release v5.4.0-BETA1#43937

Merged

Release v6.0.0-BETA1#43939

Merged

Labels

Feature FrameworkBundle Status: Reviewed

8 participants

		publicfunctioncomplete(CompletionInput$input,CompletionSuggestions$suggestions):void
		{
		if ($input->mustSuggestArgumentValuesFor('locale')) {
		$suggestions->suggestValues(Locales::getLocales());

Movatterモバイル変換

Uh oh!

[FrameworkBundle] Add completion to debug:translation command#43644

[FrameworkBundle] Add completion to debug:translation command#43644

Uh oh!

Conversation

alexandre-daubois commentedOct 22, 2021• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

GromNaN commentedOct 22, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephenkhooOct 23, 2021• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fabpotOct 25, 2021• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexandre-daubois commentedOct 22, 2021

Uh oh!

Uh oh!

Uh oh!

fabpot commentedOct 29, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

alexandre-daubois commentedOct 22, 2021•
edited
Loading

stephenkhooOct 23, 2021•
edited
Loading

fabpotOct 25, 2021•
edited
Loading