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

#63: check if inside ide installation for wrapper script #276

Closed

Conversation

mvomiero
Copy link
Contributor

@mvomiero mvomiero commented Apr 2, 2024

Fixes: #63

I haven't fully understood this part of the issue:
"Please note that you cannot output the message to std out from ideasy env --bash. So either we need to print it as error or we have to think of a different approach"

With this PR, the following is output:

  • IDE environment variables have been set for ... to stdout if inside an IDE installation
  • You are not inside an IDE installation: ... to stderr if outside an IDE installation

@VinceHeu VinceHeu assigned VinceHeu and mvomiero and unassigned VinceHeu Apr 3, 2024
@mvomiero mvomiero removed their assignment Apr 5, 2024
Copy link
Contributor

@jan-vcapgemini jan-vcapgemini left a comment

Choose a reason for hiding this comment

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

Thanks for your addition. I've just added one small CR.

if (firstCandidate == null) { // no arguments
if (current.getArgs().isEmpty()) {
info(getMessageIdeHomeFound());
return ProcessResult.SUCCESS;
Copy link
Contributor

Choose a reason for hiding this comment

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

Better remove the return, otherwise we can't show the help if no arguments were provided.

Suggested change
return ProcessResult.SUCCESS;

Copy link
Member

Choose a reason for hiding this comment

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

I do not get this change.
If the firstCandidate was not found, the first argument is not the keyword of a Commandlet.
This can happen e.g. if a add an option to a commandlet (see e.g. #279).
In such case the second for loop below handles the case to find the proper commandlet.
This however has nothing to do with the fact if IDE_HOME was found or not.

@tobka777 tobka777 closed this Apr 12, 2024
@tobka777
Copy link
Member

already fixed in #282

@mvomiero mvomiero deleted the bug/63-IdeWrapperCheckIdeInstallation branch April 12, 2024 12:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Create ide wrapper script
5 participants