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

Implemented completion aware interface for stecman symfony console completion #33

Conversation

JoshuaBehrens
Copy link
Contributor

@JoshuaBehrens JoshuaBehrens commented Apr 25, 2019

1. Why is this change necessary?

For easier usability of console commands.

Did you ever recognized a whole uuid? Do you know by heart what the plugin was spelled?

Examples in usage were already shown in the pull request for shopware 5.

2. What does this change do, exactly?

Implements for all core/shipped commands the CompletionAwareInterface
Adds hints in the documentation

3. Describe each step to reproduce the issue or behaviour.

Press tab to complete your shopware console commands. It just does not work.

4. Which documentation changes (if any) need to be made because of this PR?

https://docs.shopware.com should get a hint on the setup and pros.
There is a complementary pull request to shopware/development#20 to ensure this features is ready in the default docker container.

@JoshuaBehrens
Copy link
Contributor Author

JoshuaBehrens commented Apr 25, 2019

There is one point which needs autocompletion (in my point of view) and I am not sure how to implement it. So I'd like to have a review on the commited todo.

@mitelg
Copy link
Member

mitelg commented Apr 26, 2019

regarding the docs: you know that they are here in the sources, too? 😉 so feel free to look for the right place by yourself here: https://github.com/shopware/platform/tree/master/src/Docs/_new 😊

@JoshuaBehrens
Copy link
Contributor Author

These are the same? 😲 ok. nvm. Well I already did some changes to the docs so I guess this is already done.

@mitelg
Copy link
Member

mitelg commented Apr 26, 2019

not the whole content but the part here: https://docs.shopware.com/en/shopware-platform-dev-en

@mitelg
Copy link
Member

mitelg commented May 27, 2019

hey @JoshuaBehrens could you please rebase your branch? 😊

@JoshuaBehrens
Copy link
Contributor Author

Done.

@janbuecker
Copy link
Member

Thank you @JoshuaBehrens. I've created an internal ticket to process your PR soon. You can track the status here: https://issues.shopware.com/issues/NEXT-3676

@janbuecker
Copy link
Member

I've looked into your PR and it seems that you are missing a composer dependency which is required for your changes to work. In addition, the change in the Dockerfile will generate the completion just once for the current codebase. If you add a new command, you have to rebuild the whole container, don't you?

@JoshuaBehrens
Copy link
Contributor Author

Oh 👃 it got lost after the rebase I suppose. The change in the docker container just adds a hint for the bash_completion package to ask the application with a specific command to get autocompletion for this application. So everytime you press tab bash_autocompletion asks the console for autocompletion. So it is completely dynamically. Otherwise I had to rebuild or restart anything by implementing these interfaces.

@mitelg
Copy link
Member

mitelg commented Oct 4, 2019

hey @JoshuaBehrens
thanks for your contribution. could you please sign the CLA?

@JoshuaBehrens
Copy link
Contributor Author

CLA 😲 ? here? Since?

@mitelg
Copy link
Member

mitelg commented Oct 4, 2019

since the very beginning afaik 😄

@JoshuaBehrens
Copy link
Contributor Author

Is there a way that this is getting its way into Shopware 6.1?

@pantrtxp
Copy link
Contributor

pantrtxp commented Feb 13, 2020

Hi @JoshuaBehrens,

as you might have noticed, this feature was not included in the 6.1 :(

Given the age and the fact, that it was imported into a ticket I'm closing this issue.

Due to the issue you mentioned that our issuetracker does not inform about updates: we are currently looking into a fix for this.

Sunny Regards
Pantrtxp

@pantrtxp pantrtxp closed this Feb 13, 2020
@JoshuaBehrens
Copy link
Contributor Author

Ok now I am confused: do you want this feature or not? The target is master and therefore has not to be part of 6.1 just because I'd like to have it there. This pull request is marked as scheduled, the ticket is marked as important. Does having a ticket mean you want to implement it on your own as this code is now rejected? Do I have to keep my branch online so it can be merged later as this is marked scheduled?

When I perceive this correctly there is nothing missing here except time on your side to validate and merge this. Any rebase request might be delayed by myself but can be requested in the future.

@mitelg mitelg added Declined and removed Scheduled labels Feb 13, 2020
@pantrtxp
Copy link
Contributor

@JoshuaBehrens, sorry for this.

Please can create a new PR in regard of our new deprecation model since the master is our 6.2 development

Sunny Regards
Pantrtxp

@JoshuaBehrens
Copy link
Contributor Author

JoshuaBehrens commented Feb 13, 2020

Is there some text online that explains the deprecation model? Is it part of the contribution guidelines?

I will keep that feature alive. This is part of my daily shopware 5 work and I miss it already in shopware 6.

@pantrtxp
Copy link
Contributor

https://docs.shopware.com/en/shopware-platform-dev-en/api/api-versioning
I have just looked into you're code in detail and do not see any deprecations or breaking changes as far as I can tell (not my field of work though)

@JoshuaBehrens
Copy link
Contributor Author

Ok I will just retry it later

@JoshuaBehrens
Copy link
Contributor Author

I request a rebased form of this branch in #573

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants