git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/6] Remove return undef from ask()

From
Jeff King <peff@peff.net>
Date
May 3, 2009, 20:26 UTC
Message-ID
<20090503202625.GC20468@coredump.intra.peff.net>
In-Reply-To
<1241010743-7020-4-git-send-email-wfp5p@virginia.edu>
On Wed, Apr 29, 2009 at 09:12:20AM -0400, Bill Pemberton wrote:
> Returning undef is rarely the correct way to return a failure.
> Replace it with bare return

I'm not sure this makes sense. The function is meant to be (and is always) called in a scalar context, so "return" and "return undef" produce exactly the same results. I find the latter much more clear, because it is explicit about what the function is doing: return the user's desire; if that fails, return the default; if no default, return undef.

Returning undef can be a problem for programs which are meant to be used in list context; you generally want to return the empty list in that case (not a list with a single undef in it). So that is the reason for the advice you are quoting.

I am not sure it is worth making this function behave properly in a list context; it is not a library function, so we can see that all of the existing callers use it in a scalar context (and we would have to define sensible semantics for a list context). But even if we _did_ want to do that, your patch is only the first step. You would have to also fix the other returns which can return undef instead of an empty list. So there is not much point to your patch as it stands.

On a side note, while looking at this function, I wonder if that "return undef" is correct after all. We get there only if the user has failed to give valid input 10 times, so presumably it is a sanity check to prevent runaway input errors (and I am cc'ing Jay, who added the function not too long ago). Should we be respecting the default here, as we do when we get EOF? Although I tend to think if the user is repeatedly giving us bogus input that we should not just proceed, but should probably die. Because otherwise we are guessing at what they might have wanted.

-Peff
Previous: Jeff KingNext: Jay Soffian
Message 16 of 24 in “cleanups for git-send-email”
  1. 0/6 cleanups for git-send-emailBill Pemberton, Apr 29, 2009
  2. 1/6 Remove return undef from validate_patchBill Pemberton, Apr 29, 2009
  3. 2/6 Remove function prototypes from git-send-email.perlBill Pemberton, Apr 29, 2009
  4. 3/6 Remove return undef from ask()Bill Pemberton, Apr 29, 2009
  5. 4/6 Add explict return to end of subroutinesBill Pemberton, Apr 29, 2009
  6. 5/6 Remove mix of high and low-precedence booleansBill Pemberton, Apr 29, 2009
  7. 6/6 Remove bareword filehandles in git-send-email.perlBill Pemberton, Apr 29, 2009
  8. Jeff KingMay 3, 2009
  9. Francis GaliegueMay 3, 2009
  10. H.Merijn BrandMay 4, 2009
  11. Francis GaliegueMay 4, 2009
  12. H.Merijn BrandMay 4, 2009
  13. 5/6 Re: Remove mix of high and low-precedence booleansNicolas Sebrecht, Apr 29, 2009
  14. Bill PembertonApr 29, 2009
  15. Jeff KingMay 3, 2009
  16. Jeff KingMay 3, 2009
  17. Jay SoffianMay 4, 2009
  18. Jeff KingMay 3, 2009
  19. Jeff KingMay 3, 2009
  20. Junio C HamanoApr 29, 2009
  21. Bill PembertonApr 29, 2009
  22. Junio C HamanoApr 29, 2009
  23. 0/6 Re: cleanups for git-send-emailNicolas Sebrecht, Apr 29, 2009
  24. Andreas EricssonApr 30, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.