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

User experience improvements #2

Merged
merged 9 commits into from
Feb 26, 2024
Merged

User experience improvements #2

merged 9 commits into from
Feb 26, 2024

Conversation

jabraham17
Copy link
Member

@jabraham17 jabraham17 commented Feb 23, 2024

This PR improves the user experience of this extension by more gracefully handling errors and introducing ways of recovering from those errors

In this PR

  • major refactor of the code to be more maintainable and extendable. this refactor enabled all of the following features
  • Hides most of the default error popups from vscode, and redirects them to an error log. The it gives the user a nicer error that gives them directions on how to resolve it
  • If CHPL_HOME is not set, prompts the user for it
    Screenshot 2024-02-23 at 9 44 32 AM
    Screenshot 2024-02-23 at 9 44 39 AM
  • If chapel-py is not built, prompts the user to build it (and builds it for them)
    Screenshot 2024-02-23 at 9 45 06 AM
  • add "resolve" setting
  • fixes small bug in chapelAst syntax highlighting

[Reviewed by @ShreyasKhandekar]

@mppf
Copy link
Member

mppf commented Feb 23, 2024

If Chapel is installed with a prefix installation, there is no need to set CHPL_HOME, but chpl would be in the path.

@jabraham17
Copy link
Member Author

If Chapel is installed with a prefix installation, there is no need to set CHPL_HOME, but chpl would be in the path.

Yes, this PR searches PATH for any chpl executable, and then checks if it can find CHPL_HOME from that (since chplcheck and chpl-language-server both require it). Its mostly a heuristic, as the vscode api does not let me execute code and get back the result (so no chpl --print-chpl-home). See

export function findPossibleChplHomes(): string[] {

@mppf
Copy link
Member

mppf commented Feb 23, 2024

Right, but a --prefix installed isn't laid out that way. For example, chpl might be in /usr/bin and neither /usr nor / are "CHPL_HOME". Of course, this does not need to be fixed right now, especially if chplcheck/chpl-language-server don't work in this mode. Just something to be aware of.

Copy link
Contributor

@ShreyasKhandekar ShreyasKhandekar left a comment

Choose a reason for hiding this comment

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

This looks great to me!

Could you please update the readme so that it tracks the new features? Right now it still asks users to set CHPL_HOME explicitly.

src/ChplPaths.ts Show resolved Hide resolved
src/ChplPaths.ts Show resolved Hide resolved
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
Signed-off-by: Jade Abraham <[email protected]>
@jabraham17 jabraham17 merged commit 043d49a into chapel-lang:main Feb 26, 2024
@jabraham17 jabraham17 deleted the ux-improvements branch February 26, 2024 15:19
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.

3 participants