kr / hk

Fast Heroku client
https://hk.heroku.com/
77 stars 6 forks source link

Let app (`-a`) be a parameter to hk itself #56

Closed tt closed 10 years ago

tt commented 10 years ago

I think it would be great if the app parameter (-a) is specified right after hk itself instead of after the command. I always wonder about the order - if I should add command-specific parameters before or after the app.

I also think this aligns nicely with the extensibility model; e.g. the main Git executable allows for a --git-dir parameter which it will then provide in the environment (GIT_DIR) for sub-commands. It also aligns nicely with UNIX; the environment variable from the shell, should it exist, will be passed to sub-commands if no app parameter is specified. If the parameter is specified, this will work as a sub-shell.

I'm not that familiar with Go yet, but I'll happily try producing a pull request if there is no objections.

kr commented 10 years ago

I have some objections, but they're not that strong. Maybe medium strength, heh.

tt commented 10 years ago

The same applies to Git then. I guess the counterargument in both cases is the parameter is not something most users will provide; the client will typically be able to resolve it. In our case by using the Git remote and in Git's case by traversing the directories.

On the other hand, having the client configure the basic environment allows for some extremely concise plugins (and something like git-sh-setup.sh can be provided for various languages). Do you want a command that opens the current app in your browser? Meet hk-open:

#!/bin/sh
open "https://$HK_APP.herokuapp.com/"

(This doesn't verify the presence of the environment variable, but basic error handling is only one extra line. It also only works on OS X, but that may be okay for some plugins.)

Another way to look at it is that placing this responsibility on the main executable allows for sub-commands to be truly app-centric which most indeed are. It also makes it impossible to have different parsing of the app name in different plugins.

kr commented 10 years ago

the parameter is not something most users will provide

Unfortunately, that doesn't seem to be true, at least for Herokai and heavy users. A single git directory often has multiple heroku remotes, one for prod, staging, dev, etc.

Meet hk-open:

#!/bin/sh
open "https://$HK_APP.herokuapp.com/"

That plugin is already possible, just s/_//. See hk help plugins. ;)

The plugin still needs to do its own option parsing if it wants flags, but hk finds as much global info as it can and sets a bunch of env vars for convenience.

kr commented 10 years ago

Another way to look at it is that placing this responsibility on the main executable allows for sub-commands to be truly app-centric which most indeed are. It also makes it impossible to have different parsing of the app name in different plugins.

Yes, good points.

I do worry a little about confusion when it comes to commands that take app names but not as a flag. For example, create and destroy never look at the remote and don't use -a; they take a plain argument. A rename command would probably be similar: hk rename a b.

tt commented 10 years ago

What I meant by saying it won't get provided often was to address the point that users don't know where to place it; power users will know exactly where to put it as it is the one universal parameter. I also think it makes sense to think of it in terms of scoping, e.g, hk -a foo ps ...:

tt commented 10 years ago

The change in #57 will resolve your app like this:

  1. If a remote exists with the name of a specified app parameter, it will be translated to an app
  2. If a specified app parameter is given and no remote exists, it will be used verbatim
  3. The environment variable HKAPP treated as an app name with no translation even if a remote exists
  4. The "heroku" remote

I like this because it for power users will enable you to start a shell, write export HKAPP=foo and work on foo until you unset the environment variable. I know I often end up specifying the same app parameter for a series of commands where this would indeed improve my workflow (and my prompt could even display which app context I am on).

tt commented 10 years ago

The point about extensibility isn't that hk open can already do it, but that plugins wouldn't be able to because they have to parse app parameters themselves, so unless there's a remote named "heroku", HKAPP would be empty.

(I didn't read that you're aware of this, so just ignore this comment. I do feel that this will be easier for simple app-centric plugins.)

bgentry commented 10 years ago

@kr @tt not sure where this one stands, feel free to weigh in. Going to leave it for now.

bgentry commented 10 years ago

Oh, nevermind, this is the same as #57. Going to merge that one and close this issue then.

bgentry commented 10 years ago

Fixed with #57, thanks!