search

terminus-plugin-project / terminus-pancakes-plugin

Terminus Plugin to open Pantheon Site Databases in your Favorite SQL Client
MIT License
26 stars 20 forks source link

Added shortcut commands for all apps #10

Closed uberhacker closed 8 years ago

uberhacker commented 8 years ago

Fixed the following: PHP Notice: Undefined variable: connection_info in PancakeCommands\MySqlWorkbenchCommand.php on line 38 PHP Notice: Undefined index: sftp_port in PancakeCommands\MySqlWorkbenchCommand.php on line 105 PHP Notice: Undefined index: sftp_port in PancakeCommands\MySqlWorkbenchCommand.php on line 117

uberhacker commented 8 years ago

Just noticed these issues while testing recently.

derimagia commented 8 years ago

Fixes seem simple - for the shortcut commands can we only add them if they validated the requirements? I'd rather not clutter up people's aliases if they don't have the commands. I can take a look tomorrow probably

uberhacker commented 8 years ago

I was thinking the same thing but not sure how it can be implemented. Let me know if you figure anything out. FWIW, it does fail gracefully if, for instance, I try to run terminus site sequel in Windows.

derimagia commented 8 years ago

This isn't 100% solved yet since I didn't merge the aliases stuff in. This may be more of a question of a question for Pantheon - I think this would be better. I found a few ways I could do it including caching the validate function, dynamically requiring the files, and manually adding sub-commands but it doesn't seem to be worth it.

Sounds similar to git/drush aliases - https://github.com/drush-ops/drush/blob/0c5ec25c105205401268e69e678bd2f94772dcc9/examples/example.drushrc.php#L83

uberhacker commented 8 years ago

@derimagia: I know what you mean by not being worth it. A simple validation at the beginning of each public function should be sufficient, as long as it fails gracefully. See https://github.com/sean-e-dietrich/terminus-filer. I <3 the drush unsuck alias command since overlay was the worst ever UX design decision by Drupal. :)

derimagia commented 8 years ago

The only reason why I'm pushing back on it is because if I used that plugin I wouldn't want 5 aliases when I only needed one. That was the idea of separating them out into subcommands so that it doesn't give you more than you need/will use. But if other people request it I'll be fine with adding them in, but I'll probably dynamically add the class files as we need to if that's the case.

uberhacker commented 8 years ago

@derimagia: I totally get that. However, this seems like a lot of extra work for very little gain. If you can figure it out, then more power to you.