add use command to switch version #12

oniraf · 2019-12-08T14:40:24Z

add set command to switch between go version,
add download command to replace g install <version> --download

suggestion from #8

stefanmaric

Hi @feualpha, thanks for your contribution!

Before we continue, let's make the PR against the next branch. I already changed the PR but you will need to rebase and fix a conflict in the README

After thinking about it for a while, I've changed my mind about the approach to the "new" API:

Let's better use set instead of use. The latter sounds more like to what the current g run command does.
Let's create another command: download, which will only download the given version but not set it.
The install command, should download and set the given version without doubting (no --download flag).

To achieve this, we will need to create a new function that only downloads, let's call it download_version and rename the current download function to download_file for clarity, since that's what the function does.

Some extra bits:

As a side effect, the ACTIVATE variable should go away entirely.
The activate function which is called by the set command, should check if the version actually exists in the .versions dir and throw an "Version X is not available. Use 'g install X' to install it."
To normalize the naming of the functions, let's name/rename them set_version, install_version and download_version

The rationale here is that install should keep doing what it currently does and should be the main command users will use 99% of the time, but we should provide "porcelain" commands as escape hatches and discovery tools. Also, the deletion of the --download flag in favor of function/command composition is more elegant.

Let me know if something isn't clear

oniraf · 2019-12-14T13:57:23Z

hi @stefanmaric i have push some changes, please check it out.
also please check the command line output. i'm not sure how it should be done.

stefanmaric · 2019-12-14T15:23:35Z

check the command line output

You're right, it is a bit messed up now. Will leave you some suggestions.

stefanmaric

@feualpha thanks for your patience.

We almost done. I left a few outstanding tweaks, but otherwise this is looking pretty good.

stefanmaric · 2019-12-14T15:38:11Z

bin/g

@@ -569,6 +573,8 @@ activate() {
      done

      ln -sf "$GOROOT/bin/"* "$GOPATH/bin/"
+
+      log_info "set" "$version"


@feualpha let's remove this one.

stefanmaric · 2019-12-14T15:39:04Z

bin/g

  echo
-  log_info "installing" "$version"
+  log_info "downloading" "$version"


Let's re-rephrase to "selected"; this log has been important mainly to denote what version was resolved when using the latest keyword.

stefanmaric · 2019-12-14T15:42:43Z

bin/g

+  fi
+
+  download_version $version
+  set_version $version


@feualpha let's add a log_info here with installed and $(go version); this is mainly to confirm to the user the intended version was installed, in case something is off and the selected version is not actually activated.

I mean this bit:

if [ "$version" = "latest" ]; then version=$(get_latest_version) fi

It is being repeated quite a bit, but it is OK.

stefanmaric · 2019-12-14T15:43:59Z

bin/g

-install() {
+install_version() {
+  version=$1
+  dir="$GO_VERSIONS_DIR/$version"


Before setting this, we have to resolve the latest version in case such keyword was used. Same for the set version.

Since we would be giving false positive since $GO_VERSIONS_DIR/latest is not an actual thing.

i see. i miss this one

stefanmaric · 2019-12-14T15:53:25Z

@feualpha and also: can you squash all these changes once ready and craft a concise commit title and body to summarize the changes?

stefanmaric · 2019-12-15T14:38:41Z

bin/g

@@ -556,11 +556,15 @@ open_interactive_ui() {
 # Activate <version>
 #

-activate() {
+set_version() {
  version=$1
  active=$(get_current_version)


@feualpha only another check of latest here missing. Otherwise it looks good to merge. :)

ah yes, the consequence of exposing it

should i make it something like this? since it's already duplicated more then 3 times

resolve_version_arg(){ $version_arg=$1 if [ "$version_arg" = "latest" ]; then echo $(get_latest_version) else echo $version_arg fi } version_args=$1 version=$(resolve_version_arg "$version_arg")

@feualpha ok, let's do it. I usually avoid nesting calls too much since error handling in shell scripting is horrible, but since we are doing command substitution there already, it won't make it worse.

Let's compact it a bit:

resolve_version_argument() { if [ "$1" = "latest" ]; then get_latest_version else echo "$1" fi }

And inside the <x>_version functions, a single line:

version=$(resolve_version_argument "$1")

And @feualpha will need to you to rebase this branch on top of next since I just merged #10 which has been in the works for a while now.

This changes implement the suggestion from issue stefanmaric#8 to add specific command to switch between go version. previously it can be done by using `g` interactive mode or by using `g install <version>`. the problem comes from `g install <version>` command, because this command is also used to download the specified go version. therefore `g set <version>` is added. This changes also add `g download <version>` to download the specified go version without activating it. this command replace `g install <version> --download`. the goal of this change is to make the install function to be more conceise.

stefanmaric · 2019-12-16T07:03:56Z

@feaulpha thanks for your patience and your contribution! ❤️

I'll be releasing this along some other things sometime soon. Stay tuned!

@alvinmatias69

- Warn users about existing go installations - Improve self-upgrade script - Improve previous installation detection on g-install - Make self-upgrade throw if g was not installed via g-install - Add alias collision detection and setup helper (#11, thanks @alvinmatias69) - Add `download` and `set` commands (#12, thanks @feualpha) * BREAKING: Remove the `--download` option - Add Makefile with lint and format targets for better DX - Fix shellcheck lint errors - Format source code using make format - Improve and update docs to reflect latest contributions

stefanmaric changed the base branch from master to next December 9, 2019 12:10

stefanmaric self-requested a review December 9, 2019 13:28

stefanmaric requested changes Dec 9, 2019

View reviewed changes

stefanmaric requested changes Dec 14, 2019

View reviewed changes

oniraf force-pushed the use_command branch from 513b8f4 to aaca0fa Compare December 15, 2019 14:33

oniraf requested a review from stefanmaric December 15, 2019 14:35

stefanmaric requested changes Dec 15, 2019

View reviewed changes

oniraf force-pushed the use_command branch from a617bad to 3a1175d Compare December 15, 2019 23:30

oniraf requested a review from stefanmaric December 16, 2019 01:39

stefanmaric merged commit 70409ba into stefanmaric:next Dec 16, 2019

stefanmaric mentioned this pull request Dec 16, 2019

zsh git plugin alias #10

Closed

oniraf deleted the use_command branch December 17, 2019 00:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add use command to switch version #12

add use command to switch version #12

oniraf commented Dec 8, 2019 •

edited

Loading

stefanmaric left a comment

oniraf commented Dec 14, 2019

stefanmaric commented Dec 14, 2019

stefanmaric left a comment

stefanmaric Dec 14, 2019

stefanmaric Dec 14, 2019

stefanmaric Dec 14, 2019

stefanmaric Dec 14, 2019

stefanmaric Dec 14, 2019

oniraf Dec 15, 2019

stefanmaric commented Dec 14, 2019

stefanmaric Dec 15, 2019

oniraf Dec 15, 2019

oniraf Dec 15, 2019

stefanmaric Dec 15, 2019

stefanmaric Dec 15, 2019

stefanmaric commented Dec 16, 2019 •

edited

Loading

add use command to switch version #12

add use command to switch version #12

Conversation

oniraf commented Dec 8, 2019 • edited Loading

stefanmaric left a comment

Choose a reason for hiding this comment

oniraf commented Dec 14, 2019

stefanmaric commented Dec 14, 2019

stefanmaric left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanmaric commented Dec 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanmaric commented Dec 16, 2019 • edited Loading

oniraf commented Dec 8, 2019 •

edited

Loading

stefanmaric commented Dec 16, 2019 •

edited

Loading