Allow NoneType for CheckContext.Result

#67 Merged at 672a2af
Deleted repository
default (28ed0b62ee59)
  1. carandraug


this patch also checks for NoneType as valid result (it will print no). I stumbled upon this because GetOption will return nothing if the option was not specified. I was doing something like

def CheckEmail (context, email):
    context.Message("Checking e-mail address...")
    is_ok = GetOption ("email")
    context.Result (is_ok)
    return is_ok

Even if the patch is not accepted, someone should fix the documentation which does not mention boolean as valid.


Comments (6)

  1. Gary Oberbrunner

    Thanks, Carandraug. Could you add a test for this? Probably test/Configure/ would be a good place to add it. And of course a doc patch would also be welcome! :-)

  2. carandraug author

    Hi Gary

    a doc patch will not be necessary if this is accepted. What I meant was that this commit also adds to the documentation that a boolean is a valid class (previously undocumented) and if the fix got rejected for some reason, a separate would have to be done for it.

    Anyway, I have been thinking about this since I made the change and I wish to further change this. I may be seeing Result in the wrong light, but in my mind it's not unlike an if block and we shouldn't restrict it to certain classes. Leave it to the user to pick anything that can be evaluated as true or false and only act special in the case of a string. If you agree, I can submit another patch for something like:

    if res:
      if isistance(res, str):
        text = res
        text = "yes"
      text = "no"

    What do you think?

  3. Gary Oberbrunner

    That is better, I agree. It will also treat empty lists and hashes as "no" which I think is fine. (Alternatively, list types could throw an exception, but I think that's not necessary.) Feel free to update your pull request.

  4. carandraug author

    I have updated the pull request. What I implemented is a bit different that what I mentioned on the previous comment, it won't check for true/false if it's a string. This is compatible with the previous behaviour for strings (empty strings, display empty string, not the word "no").

  5. carandraug author

    Not really. I tried to but I can't my head around the file you mentioned. Here's an attempt at it which I though should work but does not.

    Note that removal of int() from the test that was already there should be done anyway. The patch on the pull request makes it no longer necessary.