-
-
Notifications
You must be signed in to change notification settings - Fork 295
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
Change the type of the 'immutable' parameter to boolean #651
Comments
As for the argument parsing, the following implementation would retain current CLI behaviour while producing def _immutable (s):
if s not in {'0', '1'}:
raise argparse.ArgumentTypeError(f'invalid choice: {s} (choose from 0, 1)')
return s == '1'
addarg('--immutable', type=_immutable, help=hide) …Alternatively, the following can be done instead; but it would change error output slightly (by adding single-quotes around the values). Then again, neither case prints out the "invalid int value" (can be added manually, I suppose). class _Immutable(argparse.Action):
def __call__ (self, parser, namespace, values, option_string=None):
setattr(namespace, self.dest, values == '1')
addarg('--immutable', choices={'0', '1'}, action=_Immutable, help=hide) And of course, there's also the option of using |
Don't change anything if there is no functional difference. I don't want unnecessary cosmetic changes. |
This isn't about cosmetics though – it's about the types used in the API matching their functional purpose. |
Third party utilities may already be calling this. Too late to change. |
As of now, several BukuDb methods (
add_rec()
,update_rec()
,edit_update_rec()
) accept an optional parameter namedimmutable
of integer type. This parameter is functionally boolean (signifying a yes/no condition), so I suggest changing the actual type to boolean as well.The value corresponds with bit flags, but those check for the exact value anyway, so it's perfectly possible to use the values directly when working with flags (…or better yet, a named constant:
IMMUTABLE = 0x01
).The parameter can be made nullable to account for the "noop" value (with explicit
is None
checks instead of non-transparent== -1
comparisons). …Incidentally, isn't that precisely whatOptional
is supposed to accomplish? (It's already used in most type definitions ofimmutable
… even though there doesn't seem to be anything in the code to account for it)P.S. BTW, is this flag ever used/displayed in bukuserver UI? If not, this seems like missing interop between the GUI and the library (i.e. if a user wants to edit individual bookmarks in GUI but apply batch updates via CLI… which appears to be a feature missing in the GUI as well).
The text was updated successfully, but these errors were encountered: