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

hs open command #344

Merged
merged 7 commits into from
Oct 13, 2020
Merged

hs open command #344

merged 7 commits into from
Oct 13, 2020

Conversation

drewjenkins
Copy link
Contributor

Added new hs open command. This command will open a shortcut in the user's web browser. The list so far isn't the definitive list, and can be added to as we see fit.

There are a few different ways to use hs open.

  • hs open/hs open --list - display a list of all available shortcuts
  • hs open shortcut-name - open the shortcut with name shortcut-name in your browser
  • hs open sn - open the shortcut with an alias of sn in your browser
  • hs open settings/navigation - Some links are sublinks within larger sections. Stylistically, I used / to show this. settings/navigation means that we are opening the navigation section within settings.
  • hs open 10 - open the shortcut at index 10 in your browser. I added this because I see a use case of someone doing hs open to find the shortcut they want, then just wanting the fastest way to open it. They can glance at the index and do this as shorthand. The index is subject to change if we add more commands, so there is a tiny bit of danger if someone were to do something like hs open 10 in a script, but I think that risk is pretty negligible and I would hope someone would use the full command in their script anyways.

2020-10-07 08 24 28

@anthmatic
Copy link
Contributor

Nice! My only thought is that when I ran hs open I expected to be able to select a value by index (like and inquirer.js rawlist prompt). Even though, as far as I know, we don't use that pattern in this CLI, it might be confusing. That's definitely not a deal breaker, though.

['$0 open settings/navigation'],
['$0 open sn'],
['$0 open 10'],
]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, this is cool. We should use it more.

@@ -0,0 +1,138 @@
const { getEnv } = require('./lib/config');
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should keep this in cms-cli since it is unlikely to be useful outside the CLI. Moving to CLI has the added benefit that it reduces the dependencies needed in @hubspot/cms-lib.

const { logger } = require('./logger');
const chalk = require('chalk');
const { table, getBorderCharacters } = require('table');
var opn = require('opn');
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks to be deprecated in favor of open, which we're using elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah cool, thanks. I'll change it.

@gcorne
Copy link
Contributor

gcorne commented Oct 8, 2020

hs open 10 - open the shortcut at index 10 in your browser. I added this because I see a use case of someone doing hs open to find the shortcut they want, then just wanting the fastest way to open it. They can glance at the index and do this as shorthand. The index is subject to change if we add more commands, so there is a tiny bit of danger if someone were to do something like hs open 10 in a script, but I think that risk is pretty negligible and I would hope someone would use the full command in their script anyways.

Maybe I am alone in this thought, but I think having too many ways to accomplish a task is confusing. Is there a reason that we need the numeric form?

@drewjenkins
Copy link
Contributor Author

Maybe I am alone in this thought, but I think having too many ways to accomplish a task is confusing. Is there a reason that we need the numeric form?

The use case was someone that does a hs open --list and finds the URL, the quickest way to know what to open is just by glancing at the index, however it is probably unnecessary. I'll remove.

@drewjenkins
Copy link
Contributor Author

Nice! My only thought is that when I ran hs open I expected to be able to select a value by index (like and inquirer.js rawlist prompt). Even though, as far as I know, we don't use that pattern in this CLI, it might be confusing. That's definitely not a deal breaker, though.

Ah cool, good idea I can look into that

@drewjenkins
Copy link
Contributor Author

Thanks for the feedback @gcorne and @anthmatic - incorporated both of your feedback. Took @gcorne advice and removed the ability to choose by index, and removed it from hs open --list, but took @anthmatic advice and added in Inquirer as a rawlist if the user just types hs open, which allows interactive selection either through up/down arrows or index. Seems like the best of both worlds

2020-10-08 10 34 19

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

Successfully merging this pull request may close these issues.

3 participants