Skip to content

Conversation

@halcyon
Copy link
Owner

@halcyon halcyon commented Apr 12, 2020

Fixes #51

@halcyon halcyon force-pushed the java_home branch 9 times, most recently from 1bcf5b3 to 67bc326 Compare April 12, 2020 19:50
@donbeave
Copy link

LGTM. Tried with "Oh My Zsh", works well!

Copy link

Choose a reason for hiding this comment

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

Suggested change
if [[ ! -z "${java_path}" ]]; then
if [[ -n "${java_path}" ]]; then

Copy link
Owner Author

Choose a reason for hiding this comment

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

Nice catch, completely agree - thanks!

Copy link

Choose a reason for hiding this comment

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

Suggested change
if [[ ! -z "${java_path}" ]]; then
if [[ -n "${java_path}" ]]; then

Copy link

Choose a reason for hiding this comment

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

Suggested change
if test ! -z "$java_path"
if test -n "$java_path"

Choose a reason for hiding this comment

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

Why all this complexity? I can have the correct path with asdf where java, right?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I like the way you are thinking! Resisting complexity is very important.

I did implement initially using asdf where java, but I followed the recommendation from @thuandt so we could support the system java as well.

% asdf where java
System version is selected
% asdf which java
/usr/bin/java

Copy link

@rubencaro rubencaro Apr 13, 2020

Choose a reason for hiding this comment

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

Actually, this can be done without destroying the previous value:

Suggested change
export PROMPT_COMMAND=prompt_command
export PROMPT_COMMAND="$PROMPT_COMMAND; prompt_command"

@halcyon
Copy link
Owner Author

halcyon commented Apr 13, 2020

@rubencaro @thuandt if you guys are good with it now, let me know and I'll merge

@thuandt
Copy link

thuandt commented Apr 13, 2020

@halcyon LGTM 👍

@rubencaro
Copy link

@halcyon Me too!

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.

JAVA_HOME did not get updated

5 participants