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

VsCoq downgrade message #633

Merged
merged 3 commits into from
Sep 25, 2023
Merged

VsCoq downgrade message #633

merged 3 commits into from
Sep 25, 2023

Conversation

4ever2
Copy link
Contributor

@4ever2 4ever2 commented Sep 20, 2023

Now that VsCoq 1 & 2 are different extensions on the marketplace it makes sense to point users directly to the installation page for VsCoq 1 in the error message.

Furthermore, it is now possible that both extension can be installed at the same time which is problematic. This PR attempts to detect if both extensions are enabled and warn the user that they should disable one of them.

@rtetley
Copy link
Collaborator

rtetley commented Sep 25, 2023

I have been working on something similar, except instead of pointing towards the page the action would install VsCoq Legacy and possibly disable VsCoq 2 (although I'm not sure I can do that with the vscode api). I also made the message a modal to make it clearer.

Do you think it would be better to just point towards the marketplace page ?

**
Screenshot 2023-09-25 at 09 33 01
**

@rtetley
Copy link
Collaborator

rtetley commented Sep 25, 2023

I guess pointing towards the market place page seems like the most pragmatic choice after thinking about it. What do you think about the modal message ? Once we figure that out I'll propose a change and we can merge your work :-)

@4ever2
Copy link
Contributor Author

4ever2 commented Sep 25, 2023

I think the modal message is good but maybe it should say "required for Coq <= 8.17" rather than recommended.

I don't think it is possible to disable an extension with the VS Code API, it isn't even possible to check if an extension is enabled or disabled. We can only check if it is activated, so in theory, if VsCoq 2 checks before VsCoq Legacy finishes activating then it won't warn the user of the incompatibility.

@rtetley
Copy link
Collaborator

rtetley commented Sep 25, 2023

I've kept your modifications and just turned the message into a modal, I will merge once the check passes ! Thanks a lot for your contribution :-)

@rtetley rtetley merged commit c058a99 into coq:main Sep 25, 2023
@4ever2 4ever2 deleted the vscoq-legacy-compat branch September 28, 2023 10:57
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.

2 participants