Skip to content

Conversation

@tpoliaw
Copy link
Contributor

@tpoliaw tpoliaw commented Jan 23, 2024

Improve the errors reported when a format string is not valid, eg

atuin search cargo --format '{invalid}'

gives

history output failed with: The requested key "invalid" is unknown

instead of

history output failed with: formatter error

Was the runtime-format library forked/written for use in atuin - it looks like
the timelines match? How feasible would it be either vendor the parts atuin
needs or refactor it so that the API exposes errors in a more conventional way?
I notice the repo doesn't have issues enabled or I'd raise it there.

The runtime formatting used to list history commands can fail in its
Display implementation and the error is not propagated through to the
write error it causes. Instead, the error is cached in the FormatArgs
being printed. It's a bit clumsy but by checking if there's a cached
error, we can get a more useful error for the user. eg,

    atuin search cargo --format '{invalid}'

gives

    history output failed with: The requested key "invalid" is unknown

instead of

    history output failed with: formatter error
@ellie
Copy link
Member

ellie commented Jan 24, 2024

lgtm, thanks!

Was the runtime-format library forked/written for use in atuin - it looks like
the timelines match? How feasible would it be either vendor the parts atuin
needs or refactor it so that the API exposes errors in a more conventional way?
I notice the repo doesn't have issues enabled or I'd raise it there.

I'd say that's a question for @conradludgate - are you planning on maintaining/developing it further?

Happy to vendor if needed

@ellie ellie merged commit 400e1ba into atuinsh:main Jan 24, 2024
@conradludgate
Copy link
Collaborator

I notice the repo doesn't have issues enabled or I'd raise it there.

Sorry, wasn't intentional. Enabled now

@conradludgate
Copy link
Collaborator

are you planning on maintaining/developing it further?

Yeah, I'm still maintaining it. If it's clear what exactly needs changing to make the error handling more convenient then I'm happy to make and release any changes.

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