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

Re: [PATCH 3/3] parse-options: remove PARSE_OPT_NEGHELP

From
Jeff King <peff@peff.net>
Date
Feb 28, 2012, 00:34 UTC
Message-ID
<20120228003402.GA20784@sigill.intra.peff.net>
In-Reply-To
<4F4C0308.2050804@lsrfire.ath.cx>
On Mon, Feb 27, 2012 at 11:26:16PM +0100, René Scharfe wrote:
Show 11 quoted lines
> >  OPT_REVERSE_BOOL(0, "no-index",&use_index,
> >              "finds in contents not managed by git"),
> 
> It's better than NEGHELP, but I find your use of two negations
> (REVERSE and "no-") confusing.  We don't need to invent new OPT_
> types for this, by the way, we can just do this:
> 
> 	OPT_NEGBIT(0, "no-index", &use_index,
> 	           "finds in contents not managed by git", 1),
> 
> It certainly shortens the patch.

Right. The point is that the code and the option want to look at the conditional in two different ways. So you have to have negate it _somewhere_. Either at point of use (your original patch), between the option name and the variable name (what I wrote above), or using a reversed help text (the original code before your patch).

I think we both agree that NEGHELP is the worst one. I have a slight preference for doing the reversal at the point of the option definition, if only because we know it happens at a single point, and it lets the actual code (which is usually the trickier part) be more clear. But beyond that, I think it is largely a matter of aesthetics.

Show 10 quoted lines
> >I dunno. Given that there are only two uses of NEGHELP, and that they
> >don't come out too badly, I don't care _too_ much. But I have seen some
> >really tortured logic with double-negations like this, and I'm concerned
> >that a few months down the road somebody is going to want NEGHELP (or
> >something similar) in a case where it actually does really impact
> >readability.
> 
> I'm curious to see a case that can be solved better using NEGHELP,
> but we can always add it back if we find such a beast.  I'd much
> rather see it go until then because of it's non-obvious semantics.

Yeah, I should have spelled out my "or something similar" more. I'd rather see something like NEGBIT above than NEGHELP; the latter is a bit too subtle.

-Peff
Previous: René ScharfeNext: René Scharfe
Message 17 of 19 in “parse-options: no- symmetry”
  1. 0/3 parse-options: no- symmetryRené Scharfe, Feb 25, 2012
  2. 1/3 test-parse-options: convert to OPT_BOOL()René Scharfe, Feb 25, 2012
  3. 2/3 parse-options: allow positivation of options starting, with no-René Scharfe, Feb 25, 2012
  4. Junio C HamanoFeb 26, 2012
  5. Thomas RastFeb 27, 2012
  6. Junio C HamanoFeb 27, 2012
  7. René ScharfeFeb 27, 2012
  8. Junio C HamanoFeb 27, 2012
  9. 4/3 parse-options: disallow --no-no-sthRené Scharfe, Feb 28, 2012
  10. Junio C HamanoFeb 28, 2012
  11. René ScharfeFeb 29, 2012
  12. Junio C HamanoFeb 29, 2012
  13. 3/3 parse-options: remove PARSE_OPT_NEGHELPRené Scharfe, Feb 25, 2012
  14. Jeff KingFeb 27, 2012
  15. Junio C HamanoFeb 27, 2012
  16. René ScharfeFeb 27, 2012
  17. Jeff KingFeb 28, 2012
  18. 3/3 parse-options: remove PARSE_OPT_NEGHELPRené Scharfe, Feb 28, 2012
  19. Jeff KingFeb 28, 2012

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.