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

Add setting to enable auto-ignore errors on slime-repl-history io failures #739

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Patrick-Poitras
Copy link
Contributor

We have been getting a few problems where a failure of the REPL loading/saving would prompt the user for input and some automated workflows require that this error, if it occurs, be allowed to be ignored without prompting.

This commit adds a new setting slime-repl-history-ignore-io-errors at the level of the slime-repl-call-with-handler function. This function is only used in file IO for the REPL history, so I've renamed this function slime-repl-history-call-with-handler as to make it obvious that it isn't used elsewhere. I've also cleaned up the function a little bit.

Cheers!

@stassats
Copy link
Member

stassats commented Nov 2, 2022

Silently ignoring errors doesn't sound great.

Thanks to @stassats for helpful suggestions on improving the interface
@Patrick-Poitras
Copy link
Contributor Author

@stassats Yeah, I think that most users would like to have an error message at least tell them if the situation happened. It's not something our (i.e. internal to my workplace) users care about, but as a general interface, it's not great that it would immediately default to ignoring error messages.

I've made a second version that warns the user if it happens. I'm much happier with it from an UX perspective, thank you so much for the suggestion.

PS. Sorry for the multiple overwriting builds in the last hour, there was an error that triggered on CCL but not on SBCL and I was trying to see if it had to do with the function return being different. Forgot this is connected and updates automatically. My apologies if you got pinged 50 times.

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.

None yet

2 participants