fix #141 and catch conversion errors when updating settings #142

Merged
merged 2 commits into from Mar 23, 2013

Projects

None yet

3 participants

@rmartinjak

See #141

@ppurka ppurka commented on the diff Mar 22, 2013
sagenb/notebook/conf.py
@@ -107,10 +107,16 @@ def update_from_form(self, form):
val = False
elif typ == T_INTEGER:
- val = int(val)
+ try:
+ val = int(val)
+ except ValueError:
+ val = self[key]
@ppurka
ppurka Mar 22, 2013 Sage Mathematical Software System member

Does the element self.confs[key] always exist? It isn't obvious from the file. If it doesn't always exist, is it possible to set val to some other default?

I did notice that by default it will return the value of defaults[key] from server_conf.py. Is it possible for the key to be not present there?

@rmartinjak
rmartinjak Mar 22, 2013

I'd say anybody that adds a new option should also add a default value.

And line 122 (pre-commit) / 128 (post-commit) already uses self[key] anyway.

@ppurka
Member
ppurka commented Mar 23, 2013

OK. Then. Let's merge this. I don't see any other problems with this patch.

@kini kini merged commit a2eb724 into sagemath:master Mar 23, 2013
@kini
Member
kini commented Mar 23, 2013

Done, thanks!

@rmartinjak rmartinjak deleted the rmartinjak:issue141 branch Mar 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment