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.

Sign up for GitHub

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

Jump to bottom

Patch: Better error reporting when a watched path does not exist #329

Merged
merged 1 commit into from
Dec 3, 2024
Merged

Patch: Better error reporting when a watched path does not exist #329

merged 1 commit into from
Dec 3, 2024

Conversation

Xophmeister
Copy link
Contributor

Provide a more helpful error message when a watched path does not exist. For example,

$ ghciwatch --watch does-not-exist

Before

Error:   × Tasks failed:
  │ • run_ghci: ghci event channel closed
  │ • run_watcher: No such file or directory (os error 2)

After

Error:   × Tasks failed:
  │ • run_ghci: ghci event channel closed
  │ • run_watcher: Cannot watch path that doesn't exist: "/home/chris/Projects/tweag/ghciwatch/does-not-exist"

Partially resolves #312

  • Labeled the PR with patch, minor, or major to request a version bump when it's merged.
  • Updated the user manual in docs/.
  • Added integration / regression tests in tests/.

@github-actions github-actions bot added the patch Bug fixes or non-functional changes label Nov 5, 2024
@smelc
Copy link
Contributor

smelc commented Nov 8, 2024

@9999years> any feedback you would have on this PR? (so that we know if there is a chance it gets merged, and if so we'll work on reaching a final state (add a test?)).

9999years
9999years reviewed Nov 26, 2024
View reviewed changes
src/watcher.rs Outdated Show resolved Hide resolved
9999years
9999years reviewed Nov 26, 2024
View reviewed changes
src/watcher.rs Outdated Show resolved Hide resolved
@9999years
Copy link
Member

@Xophmeister @smelc This is awesome, sorry I didn't see it earlier! I've been heads-down working on upgrading our monolith to GHC so my notifications have been stalled. Looks good! Left a couple comments but otherwise this should be a simple fix.

You might be able to write a test for it but the testing framework is a little clunky. Not required.

@Xophmeister
Copy link
Contributor Author

Thanks, @9999years 🙏

The original issue (#312) mentions two actions items:

Action items here:

  • Better error message when the --watch path isn't found.

  • --verbose and/or --debug flags as shortcuts for --log-filter ghciwatch=debug,info or similar.

Would your preference be for this PR to close 312 (thus needing the second item), or is this enough as is?

@9999years
Copy link
Member

This will close #312, I've filed #331 for the --verbose/--debug flags as those are tangential.

@9999years
Copy link
Member

Aside, this part of the message is still really clunky and unhelpful. Don't fix it in this PR but I just want to flag this as something that still needs work / isn't user-friendly enough:

Error:   × Tasks failed:
  │ • run_ghci: ghci event channel closed

@9999years
Copy link
Member

Anything left before merge? I see it's still marked as a draft.

@smelc
Copy link
Contributor

smelc commented Nov 26, 2024

@Xophmeister> WDYT? Ready to merge in your opinion?

@Xophmeister Xophmeister marked this pull request as ready for review November 27, 2024 09:00
@smelc
Copy link
Contributor

smelc commented Nov 28, 2024

@9999years> it's ready to go 🎉

@smelc smelc force-pushed the smelc/better-error-report-when-watching-notfound branch from 7bc0e60 to 96dcf36 Compare November 29, 2024 08:56
@smelc
Copy link
Contributor

smelc commented Nov 29, 2024

Squashed to have a nicer history.

@9999years 9999years merged commit b998644 into MercuryTechnologies:main Dec 3, 2024
39 checks passed
9999years added a commit that referenced this pull request Dec 3, 2024
@9999years
Fix release workflow with PRs from forks
f681b5a 
PR #329 failed to create a release because the release workflow ran in
the context of the fork:

```
Run peter-evans/create-pull-request@v5
Error: Input 'token' not supplied. Unable to continue.
Restore git configuration
  Error: Cannot read properties of undefined (reading 'removeAuth')
```

Change the workflow to run in `pull_request_target` to fix this, I hope.

Also update `get-crate-version` to not depend on any runtime tools, only
the Nix expressions.

See: https://github.com/MercuryTechnologies/ghciwatch/actions/runs/12130981516/job/33822389311
@9999years 9999years mentioned this pull request Dec 3, 2024
Merged
9999years added a commit that referenced this pull request Dec 3, 2024
@9999years
Fix release workflow with PRs from forks (#332)
2d61403 
PR #329 failed to create a release because the release workflow ran in
the context of the fork:

```
Run peter-evans/create-pull-request@v5
Error: Input 'token' not supplied. Unable to continue.
Restore git configuration
  Error: Cannot read properties of undefined (reading 'removeAuth')
```

Change the workflow to run in `pull_request_target` to fix this, I hope.

Also update `get-crate-version` to not depend on any runtime tools, only
the Nix expressions.

See:
https://github.com/MercuryTechnologies/ghciwatch/actions/runs/12130981516/job/33822389311
@9999years
Copy link
Member

Released this in 1.0.2: https://github.com/MercuryTechnologies/ghciwatch/releases/tag/v1.0.2

Thanks again for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@9999years 9999years 9999years approved these changes

Assignees
No one assigned
Labels
patch Bug fixes or non-functional changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"run_ghci: ghci event channel closed" and "run_watcher: No such file or directory (os error 2)"
3 participants
@Xophmeister @smelc @9999years