Skip to content

Conversation

@qexat
Copy link
Collaborator

@qexat qexat commented Feb 22, 2025

Fixes #55

Before this commit, the function that handled invoking the cloning command did not perform any error handling.
This PR attempts to fix that ; this means that Shell.proc now returns a result instead of unit, which is a breaking change. As such, the code that depends on this function - such as clone_repo in Tui.Init was updated to handle the result and exit with a better error message (well, at least one that is more useful).

Things that I am still not convinced about in this fix:

  • The return type of Shell.proc is (unit, string) result. I am not really sure about crafting the error message there.
  • clone_repo currently exits, but since it is an "internal" function (it is not exposed in the interface of the module), it is probably safer to make it return a result as well (as it is fallible) and let a public function exit on failure.

@qexat qexat added type: bug Something isn't working component: UI Issues related to changing UI labels Feb 22, 2025
Copy link
Collaborator

@heathhenley heathhenley left a comment

Choose a reason for hiding this comment

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

Nice! Just ran it and in IMO it's a way more obvious what's going on.

Copy link
Owner

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

This implementation is super neat! I learned something new here. Thanks a lot for contributing this fix!

@chshersh chshersh merged commit a0d6f13 into main Feb 26, 2025
3 checks passed
@chshersh chshersh deleted the fix-55 branch February 26, 2025 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: UI Issues related to changing UI type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Exception raised by Cmdliner: Invalid_argument("index out of bounds")

4 participants