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

Split HeidiSQL and Sequel Pro commands into separate class files #7

Closed uberhacker closed 8 years ago

uberhacker commented 8 years ago

Added MySQL Workbench support

derimagia commented 8 years ago

Very awesome! Looks great, thanks for the work!

I would have preferred to keep the pancakes command and keep them all under that so we don't add a lot of commands for people who don't use the other apps - but if people want them separate I don't mind being overruled on that.

Best proposition I have is to add one more command (the original pancakes command) back that will "auto detect" your program and then you can override it with an option or an environment variable. Then we wrap all of the new classes and only create them if you have the app installed (so it doesn't init the class if you don't have mysql workbench installed). Thoughts @uberhacker @rvtraveller ?

uberhacker commented 8 years ago

Hey Dave, That sounds reasonable to me. Sorry about nixing the original pancakes command. I just didn't understand its purpose until now. Thanks for the explanation. Makes sense. Thanks

rvtraveller commented 8 years ago

@derimagia I agree that having just 1 command is better. With multiple commands folks have to figure out the "translation" as they talk to people on other platforms. Having the environment variable is a nice way for people to provide a static choice if they have multiple SQL programs installed.

uberhacker commented 8 years ago

I might also add we should not assume all Windows users will install apps on the C: drive. Also, terminus does an awful job of not letting you know when it can't find an app. If we can figure out a way to let the user know their app cannot be found, that would helpful. It's easy to mistype a character when setting the path or an environment variable. What are your thoughts of extending the PancakesCommand class instead of TerminusCommand?

derimagia commented 8 years ago

I ended up spending some time on this and I like how it is so far:

https://github.com/derimagia/terminus-pancakes/tree/pancakes-plugins

Pretty much there are sub pancakes commands now and each one has "validate" and "execute" functions. If it doesn't validate it won't be a candidate.

To be specific on an app you can use "--app=" and then set an alias defined in the plugin file. I moved most of the code to the parent class.

Can you guys take a look at Heidi/Mysqlworkbench? I wasn't able to completely test them so they may need a little bit of work

uberhacker commented 8 years ago

@derimagia: Nice work with the reflection abstraction. I have rolled your changes into this PR plus some fixes to get MySQL Workbench working in Windows. Please check it out when you get a chance.

derimagia commented 8 years ago

Awesome, I think it's just heidi left to check then so hopefully we can get that done in the next few days. I removed the heidi variable since I was playing with the idea of grabbing the user's path but we can just add it back in, good work

uberhacker commented 8 years ago

Heidi was working fine in Windows when I tested. MySQL Workbench works in Linux. Probably should test Sequel Pro and MySQL Workbench again on the Mac. I have Mountain Lion running in a Virtualbox environment, so my testing is not with the most recent Mac OS X. Also, Xcode didn't install correctly so that should give you a good indication of my Mac OS testing reliability.

derimagia commented 8 years ago

I merged this for now since it's mainly working. Few things.

1) First of all thanks for the awesome work 2) I changed it so that mac/linux checks which even if it's not using the --app flag since this would mean you wouldn't be able to launch it by default otherwise - if you were thinking about performance we can probably cache it later on, but it only runs when you run the command so it's fine for now I think 3) It looks like the workbench support doesn't work if you haven't make settings in mysqlworkbench, it didn't work for me on the mac because of this but it did start it up. Can you also document why it doesn't work on the latest version or at least post it here?

uberhacker commented 8 years ago

@derimagia: Sure. The main reason the latest version of MySQL Workbench for Mac doesn't work is because they changed the way the configuration settings are stored. If we can test for different versions, I suppose we could make it work but oddly enough, this is only on Mac. Linux and Windows store the same way (as version 6.2.5 for Mac). Strange that they would change it only on the Mac for the same version.