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

Update install_macos.sh #6390

Closed
wants to merge 2 commits into from
Closed

Update install_macos.sh #6390

wants to merge 2 commits into from

Conversation

agjftucker
Copy link
Collaborator

@agjftucker agjftucker commented Aug 5, 2023

The basic download-and-drag installation of VS Code does not add it to the PATH. So this commit calls it with its full pathname.

This script-driven install is the one advocated on the leanprover-community website and I am trying to make it a little smoother. It should work for both Intel and M1-based Macs and it would be great if the install page were to change to reflect that (see here). However I don't really understand why we diverge at all from the approach described in the Lean manual.


Open in Gitpod

The basic Mac installation of VS Code does not add it to the PATH.
@agjftucker agjftucker added the awaiting-review The author would like community review of the PR label Aug 5, 2023
Copy link
Contributor

@ocfnash ocfnash left a comment

Choose a reason for hiding this comment

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

This is brittle to the situation that VS Code is not in the expected location, right?

I think we could live with that but it would be nice if there was a way to ask Mac OS where it is (if it exists).

@@ -16,9 +16,8 @@ elan toolchain install stable
elan default stable

# Install and configure VS Code
if ! which code > /dev/null; then
if ! "/Applications/Visual Studio Code.app/Contents/Resources/app/bin/code" --install-extension leanprover.lean4; then
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if ! "/Applications/Visual Studio Code.app/Contents/Resources/app/bin/code" --install-extension leanprover.lean4; then
if ! "/Applications/Visual Studio Code.app/Contents/Resources/app/bin/code" --install-extension leanprover.lean4 > /dev/null; then

brew install --cask visual-studio-code
# Install the Lean4 VS Code extension
code --install-extension leanprover.lean4
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we also provide the full path here?

Suggested change
code --install-extension leanprover.lean4
"/Applications/Visual Studio Code.app/Contents/Resources/app/bin/code" --install-extension leanprover.lean4

Given this I guess we should capture this in a variable above.

@ocfnash ocfnash added awaiting-author A reviewer has asked the author a question or requested changes and removed awaiting-review The author would like community review of the PR labels Aug 24, 2023
@agjftucker agjftucker closed this Aug 24, 2023
@agjftucker agjftucker deleted the agjftucker-patch-1 branch August 24, 2023 13:10
@ocfnash
Copy link
Contributor

ocfnash commented Aug 24, 2023

@agjftucker Did you close this because we were too slow reviewing? If so, I'm sorry!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-author A reviewer has asked the author a question or requested changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants