Embedded PHPCR Shell #151

dantleech · 2014-06-05T15:23:20Z

Replaces existing commands with embedded PHPCR Shell.

For example

$ php app/console doctrine:phpcr:shell node:list / # commands with arguments can be entered without quotes
$ php app/console doctrine:phpcr:shell "ls -L2" # commands with options must be quoted
$ php app/console doctrine:phpcr:shell node-type:list
$ php app/console doctrine:phpcr:shell "SELECT * FROM [nt:unstructured]" # queries must be quoted

Or launch a shell session

$php app/console doctrine:phpcr:shell

stof · 2014-06-05T15:37:14Z

Command/PhpcrShellCommand.php

you need to handle the exit code of the embedded application, to reutnr it as the exit code of the command

stof · 2014-06-05T15:37:50Z

removing the existing commands is a BC break. I'm not sure such cleanup justifies a bump to 2.0 now. you may keep the old commands around (deprecating them instead of removing them)

dantleech · 2014-06-05T15:42:07Z

@stof yeah the PR in its current state would be a major version change, we could also conditionally support the phpcr-shell only if it is installed.

stof · 2014-06-05T15:43:28Z

@dantleech my suggestion is to keep the existing command and to add the new phpcr shell command => no BC break but a new feature => next minor version.

stof · 2014-06-05T15:44:24Z

and the composer.json needs to be updated to require the PHPCR shell if you use it

dbu · 2014-06-11T11:23:41Z

i agree with stof: lets keep the old commands for now but deprecate them and start requiring the shell for the 1.2 version. the commands will be unmaintained but can stay for a while, but surely wont get any new features.

it would be really nice if we can disable parsing the arguments, or as a workaround have the phpcr command expect any parameters and forward them, if that is possible.

stof · 2014-06-11T12:31:52Z

I don't think that's possible

dantleech · 2014-06-11T12:36:53Z

Its possible if we can change the InputInterface class. (this is what I do in PHPCR shell to allow phpcr:query:select FROM [nt:foobar] blah blah blah). But I can only do that because I use my own Application class.

dantleech · 2014-06-14T05:44:39Z

I have mitigated the quoting problem somewhat by making the argument an array, concatenating the values with space and then passing this to the nested command. If options are needed then the entire command still needs to be quoted however.

dantleech · 2014-06-14T05:45:23Z

Command/PhpcrShellCommand.php

Argument is now an array

dantleech · 2014-06-14T06:01:02Z

Reverted the removal of the legacy commands, updated help and added suggest to composer. If the phpcr-shell package is not installed, the command will throw an exception.

dantleech · 2014-06-14T06:04:07Z

Command/PhpcrShellCommand.php

Argument is now an array

dbu · 2014-06-14T08:54:54Z

looks good! can you squash the commits please? and update the symfony-cmf-docs (dev branch) with some documentation and a link to the shell?

btw, if i don't specify a command for the shell, do i get into the interactive shell with this command?

dbu · 2014-06-14T08:55:30Z

Command/PhpcrShellCommand.php

... SELECT SQL2 queries ...

dantleech · 2014-06-14T10:19:13Z

Re. launching the shell, thats something I should add, but not sure how - spec

dantleech · 2014-06-14T10:21:33Z

Ifying nothing lists all the commands, and adding a --shell option might be the way forward.

Btw. This also beeds a a PR merge on the shell.

dbu · 2014-06-15T11:48:30Z

+1 for a --shell option. or maybe --interactive / -i ?

i changed the heading to WCM to remind us we wait for the shell PR to be merged.

stof · 2014-06-16T07:29:23Z

--shell is already used by Symfony. And --interactive could be confusing with --no-interaction used by Symfony

dbu · 2014-06-16T08:03:36Z

gnarc. what if we say without arguments you go into the interactive shell and you need to run list to get a list of commands? otherwise, --cli ?

dantleech · 2014-06-16T11:36:09Z

I think without arguments for the shell is OK - its probably the preferred way to execute commands unless you are scripting.

dantleech · 2014-06-16T19:13:27Z

ok updated.

Shell is now launched as the default action ./app/console phpcr
- When launched like this it will create a configuration directory in the users home folder. Does that matter?
The "deprecated" commands will be unavailable when phpcr-shell is present.
The phpcr command will be present all the time however, to advertise its existence.

dantleech · 2014-06-16T19:15:12Z

Command/NodeDumpCommand.php

Unfortunately this is copy/pasted in each command, we can't use inheritance and we can't use traits due to BC.

i am -1 on this. not due to copy-paste but because it in effect means a BC break for the user. he can't use the command he was used to anymore. i think we should just update the description to append the deprecated info, but otherwise leave them in until we do a phpcr-odm 2.0

I agree with @dbu

Ok.. I reasoned that the user explicitly adds PHPCR-Shell to their dependencies, and only then do the commands disappear. So its an option. Is that still a BC break?

Christophe Coevoet [email protected] wrote:

@@ -94,4 +94,12 @@ protected function execute(InputInterface $input,
OutputInterface $output)

return parent::execute($input, $output); }

/**

\* Only enable this command if PHPCR Shell is not installed.

*/

public function isEnabled()

{

return !DoctrineCommandHelper::isPhpcrShellInstalled();

I agree with @dbu

Reply to this email directly or view it on GitHub:
https://github.com/doctrine/DoctrinePHPCRBundle/pull/151/files#r13833585

Sent from my Android device with K-9 Mail. Please excuse my brevity.

yes. If you have a deploy script using the existing command, you might still want to add PHPCR-Shell in your project to get the new features without breaking your deployment for instance

Yeah just ran into that very problem :)

dantleech · 2014-06-16T19:18:34Z

This PR should also wait until PHPCR-Shell has some issues fixed - e.g. the table formatting seems broken in embedded mode, and also in my test project i am using Symfony 2.5 and phpcr-shell there is a BC break (also in the TableHelper)

dbu · 2014-06-16T21:38:11Z

cool with the default shell. about creating a file: what is the point of that? history of commands? global configuration? maybe it would make more sense to create that file in the project root, or even the app/cache/ , depending on what exactly it does.

dantleech · 2014-08-07T05:38:28Z

I will try and get this ready for the 1.2 release.

@dbu the configuration folder currently stores two types of configuration: aliases (e.g. ls: node:list, cp: node:copy, use: workspace:use, explain: node-type:show, etc.) and connection profiles.

Connection profiles will not be created in the embedded mode, the alias file is always created however, its global configuration. (and can be reinitialized / reloaded with shell:config:reload and shell:config:init).

Personally I don't see any reason to disable this, but still feels wierd - one option might be to read the file if it is there but never create it.

dbu · 2014-08-07T07:15:06Z

i would expect the aliases to be read from symfony configuration in embedded mode. additionally reading the config file if it happens to be there seems ok to me. that way a user can share his alias between projects, or have his preferred personal aliases. precedence in case of name conflicts is a bit tricky, not sure what makes more sense.
but i would avoid creating the folder in embedded mode.

btw, this needs a rebase.

dantleech · 2014-08-07T08:53:19Z

Reading aliases from the SF configuration might be interesting, e.g. exportcms: session:export myexport.xml /cms/foobar/blocks for instance. I would say then that.

Read the global configuration if it exists, if not use the default (but do not write to the FS).
Merge any aliases defined in the Bundle

Will need some refactoring on the Shell PR, but sounds good.

dbu · 2014-08-07T09:23:36Z

:y:

dantleech · 2014-08-09T22:09:28Z

ok. I have updated and rebased this PR and the corresponding PHPCR-Shell PR has been updated also.

Regarding config creation, in shell mode a config file will never be created automatically, but if the configuration is there it will be used. (the user can still manually create the config with shell:config:init). Otherwise the default dist configuration will be loaded.

I decided against adding configuration for project specific shell configuration at this point.

So, for me this is good.

dantleech · 2014-08-09T22:11:14Z

Also I changed the command from $ app/console phpcr to app/console phpcr:shell as I don't think people expect it to be at the top level.

dbu · 2014-08-11T08:13:34Z

Command/PhpcrShellCommand.php

this is now phpcr:shell, no?

use %command.full_name% in the help text instead of app/console phpcr:shell. Symfony does the replacement automatically (there is also ``%command.name%for just thephpcr:shell` part). this way, you don't need to change the help when the command name changes (or when the user renames the `app/console` file to something else)

Done as @stof said, and changed the name to doctrine:phpcr:shell -- which I think is the most correct.

- Added `doctrine:phpcr:shell` command to launch either single commands or start a shell session based on the default or given doctrine session.

dantleech · 2014-08-14T18:10:28Z

Updated and squashed and I have merged the related PHPCR-Shell PR.

dbu · 2014-08-15T10:17:00Z

+1 to merge. @lsmith77 happy too?

we should add a note to the symfony-cmf-docs somewhere. btw, you could also check if you find a place in phpcr-odm-documentation to point to the shell, as its useful in standalone phpcr-odm usage as well.

Embedded PHPCR Shell

stof reviewed Jun 5, 2014
View reviewed changes

dantleech mentioned this pull request Jun 6, 2014

Added ability to import a file into the repository phpcr/phpcr-shell#34

Merged

dantleech reviewed Jun 14, 2014
View reviewed changes

Command/PhpcrShellCommand.php Outdated

Copy link

Contributor Author

dantleech Jun 14, 2014

Choose a reason for hiding this comment

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

Argument is now an array

dantleech reviewed Jun 14, 2014
View reviewed changes

Command/PhpcrShellCommand.php Outdated

Copy link

Contributor Author

dantleech Jun 14, 2014

Choose a reason for hiding this comment

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

Argument is now an array

dbu reviewed Jun 14, 2014
View reviewed changes

Command/PhpcrShellCommand.php Outdated

Copy link

Member

dbu Jun 14, 2014

Choose a reason for hiding this comment

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

... SELECT SQL2 queries ...

dbu changed the title ~~[RFC] Embedded PHPCR Shell~~ [WCM] Embedded PHPCR Shell Jun 15, 2014

dantleech reviewed Jun 16, 2014
View reviewed changes

dantleech mentioned this pull request Jul 8, 2014

The node dump does not show associative array keys phpcr/phpcr-utils#117

Closed

dbu reviewed Aug 11, 2014
View reviewed changes

Embedded PHPCR Shell

0668fdd

- Added `doctrine:phpcr:shell` command to launch either single commands or start a shell session based on the default or given doctrine session.

dbu changed the title ~~[WCM] Embedded PHPCR Shell~~ Embedded PHPCR Shell Aug 15, 2014

lsmith77 added a commit that referenced this pull request Aug 15, 2014

Merge pull request #151 from dantleech/embedded_phpcr_shell

e1a559c

Embedded PHPCR Shell

lsmith77 merged commit e1a559c into doctrine:master Aug 15, 2014

dantleech mentioned this pull request Oct 23, 2014

news for cmf 1.2 stable symfony-cmf/symfony-cmf-website#59

Merged

Embedded PHPCR Shell #151

Embedded PHPCR Shell #151

Uh oh!

Conversation

dantleech commented Jun 5, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stof commented Jun 5, 2014

Uh oh!

dantleech commented Jun 5, 2014

Uh oh!

stof commented Jun 5, 2014

Uh oh!

stof commented Jun 5, 2014

Uh oh!

dbu commented Jun 11, 2014

Uh oh!

stof commented Jun 11, 2014

Uh oh!

dantleech commented Jun 11, 2014

Uh oh!

dantleech commented Jun 14, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dantleech commented Jun 14, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dbu commented Jun 14, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dantleech commented Jun 14, 2014

Uh oh!

dantleech commented Jun 14, 2014

Uh oh!

dbu commented Jun 15, 2014

Uh oh!

stof commented Jun 16, 2014

Uh oh!

dbu commented Jun 16, 2014

Uh oh!

dantleech commented Jun 16, 2014

Uh oh!

dantleech commented Jun 16, 2014

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dantleech commented Jun 16, 2014

Uh oh!

dbu commented Jun 16, 2014

Uh oh!

dantleech commented Aug 7, 2014

Uh oh!

dbu commented Aug 7, 2014

Uh oh!

dantleech commented Aug 7, 2014

Uh oh!

dbu commented Aug 7, 2014

Uh oh!

dantleech commented Aug 9, 2014

Uh oh!

dantleech commented Aug 9, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!