search

pazz / alot

Terminal-based Mail User Agent
GNU General Public License v3.0
697 stars 165 forks source link

Fix configobj to relay tuple type errors #1633

Closed brandonpayton closed 10 months ago

brandonpayton commented 10 months ago

Some current config tuple checking is of the form:

if not isinstance(value[1], int) or not isinstance(value[2], int):
    VdtTypeError(value)

This has two problems. The first is that isinstance('123', int) is always False, which means the if's block always runs. A VdtTypeError would always be raised except that the raise keyword is missing.

This PR attempts to fix that issue.

lucc commented 10 months ago

This width_tuple function looks quite simple. Do you mind adding some tests to tests/utils/test_configobj.py? Preferably also for the error handling that you improved (i.e. asserting that the right exceptions are thrown).

brandonpayton commented 10 months ago

I committed some tests but need to break them into smaller pieces and add one or two more.

brandonpayton commented 10 months ago

Thanks also for taking a look at this.

brandonpayton commented 10 months ago

OK, I added some width_tuple tests, made the raised exceptions more precise, and consolidated the exception handling. This should be ready for another look.

brandonpayton commented 10 months ago

That is much cleaner. Thank you!

lucc commented 10 months ago

Thank you for your contribution :+1: