search

jorgerojas26 / lazysql

A cross-platform TUI database management tool written in Go.
MIT License
916 stars 51 forks source link

Do not try to recover from panics, but use the return values #133

Closed svanharmelen closed 4 days ago

svanharmelen commented 4 days ago

Sorry for all the PRs, I'm just having fun with it and am hitting a lot of small issues when using lazysql which I would like to help improve 😉

This PR removes all the calls to recover as it is very discouraged (see Go docs) to try and recover panics as in in 99.999999% of the cases that indicates a bug in the program or another error not being handled properly which should be fixed instead.

In order to take any actions using a defer that will only do something if the function returned an error, its best to just check against that error. While in Go its no too common to use named return values in the function signature, in the case where you want a defer to have access to the actual returned value I found it to be the most robust to at least name the error in the function signature. But in this case I noticed you already named all return values so I didn't need to change anything for that.

jorgerojas26 commented 4 days ago

That is fair enough :) just fix the linter issues and i'll merge.

jorgerojas26 commented 4 days ago

Thanks for all the PRs, by the way. I'm just a bit busy right now, but i'll try to follow all of them.

svanharmelen commented 4 days ago

The linter issue is fixed by #132 I think the linter updated as I didn't tough that code...

jorgerojas26 commented 4 days ago

There are other linter issues in ResultsTable.go:814 and ConnectionsTable.go:77

svanharmelen commented 4 days ago

Ah sorry, missed that one! Should be fixed now...

svanharmelen commented 4 days ago

Addressed all feedback and rebased all PRs so they pass the linter again 👍🏻