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

Re: [PATCH v2 2/3] range-diff/format-patch: handle commit ranges other than A..B

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Jan 28, 2021, 15:38 UTC
Message-ID
<nycvar.QRO.7.76.6.2101281636400.54@tvgsbejvaqbjf.bet>
In-Reply-To
<xmqqa6sty55k.fsf@gitster.c.googlers.com>
Hi Junio,
On Wed, 27 Jan 2021, Junio C Hamano wrote:
Show 25 quoted lines
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > Maybe I am too stuck with the idea of avoiding regular expressions after
> > that StackOverflow incident... Maybe
> >
> > 	static regex_t *regex;
> >
> > 	if (strstr(range, ".."))
> > 		return 1;
> >
> > 	if (!regex) {
> > 		regex = xmalloc(sizeof(*regex));
> > 		if (regcomp(regex, "\\^(!|-[0-9]*)$", REG_EXTENDED))
> > 			BUG("could not compile regex");
> > 	}
> >
> > 	return !regexec(regex, range, 0, NULL, 0);
> >
> > is more readable, and acceptable in this context?
>
> Readable, yes, acceptable, I don't know, and I am not even sure if I
> want to be the one to judge to be honest ;-)
>
> Have you tried the approach to feed the thing to setup_revisions()
> and inspect what objects are in revs.pending?
I hadn't thought of that.
Show 8 quoted lines
> When you got a valid range, you should find one or more positive and
> one or more negative commits , and the approach won't be fooled by
> strings like "HEAD^{/other than A..B/}".
>
> Or does the revision parsing machinery too eager to "die" when we
> find a syntax error?  If so, scratch that idea, but in the longer
> haul, fixing these die's would also be something we'd want to do to
> make more parts of libgit.a callable as a proper library.

It should probably be libified anyway if it calles `die()` (I don't know off the top of my head).

Worse than the `die()` is probably the allocated memory; IIRC there is no way to release the memory allocated by `setup_revisions()` unless one performs a revwalk.

Will try to find some time to play with the `setup_revisions()` idea.

Ciao, Dscho

Previous: Junio C HamanoNext: Johannes Schindelin via GitGitGadget
Message 27 of 63 in “Range diff with ranges lacking dotdot”
  1. 0/3 Range diff with ranges lacking dotdotJohannes Schindelin via GitGitGadget, Jan 21, 2021
  2. 1/3 range-diff: refactor check for commit rangeJohannes Schindelin via GitGitGadget, Jan 21, 2021
  3. Junio C HamanoJan 21, 2021
  4. Phillip WoodJan 22, 2021
  5. Junio C HamanoJan 22, 2021
  6. Phillip WoodJan 23, 2021
  7. Johannes SchindelinJan 26, 2021
  8. 2/3 range-diff: handle commit ranges other than A..BJohannes Schindelin via GitGitGadget, Jan 21, 2021
  9. Eric SunshineJan 21, 2021
  10. Johannes SchindelinJan 22, 2021
  11. Junio C HamanoJan 21, 2021
  12. Johannes SchindelinJan 22, 2021
  13. 3/3 range-diff(docs): explain how to specify commit rangesJohannes Schindelin via GitGitGadget, Jan 21, 2021
  14. Junio C HamanoJan 21, 2021
  15. Johannes SchindelinJan 22, 2021
  16. Junio C HamanoJan 22, 2021
  17. Johannes SchindelinJan 27, 2021
  18. Junio C HamanoJan 28, 2021
  19. Uwe Kleine-KönigJan 22, 2021
  20. Johannes SchindelinJan 26, 2021
  21. Uwe Kleine-KönigJan 22, 2021
  22. 0/3 Range diff with ranges lacking dotdotJohannes Schindelin via GitGitGadget, Jan 22, 2021
  23. 2/3 range-diff/format-patch: handle commit ranges other than A..BJohannes Schindelin via GitGitGadget, Jan 22, 2021
  24. Junio C HamanoJan 22, 2021
  25. Johannes SchindelinJan 27, 2021
  26. Junio C HamanoJan 28, 2021
  27. Johannes SchindelinJan 28, 2021
  28. 3/3 range-diff(docs): explain how to specify commit rangesJohannes Schindelin via GitGitGadget, Jan 22, 2021
  29. 1/3 range-diff/format-patch: refactor check for commit rangeJohannes Schindelin via GitGitGadget, Jan 22, 2021
  30. Junio C HamanoJan 22, 2021
  31. Uwe Kleine-KönigJan 25, 2021
  32. Junio C HamanoJan 25, 2021
  33. Uwe Kleine-KönigJan 25, 2021
  34. Junio C HamanoJan 26, 2021
  35. 0/3 Range diff with ranges lacking dotdotJohannes Schindelin via GitGitGadget, Jan 27, 2021
  36. 3/3 range-diff(docs): explain how to specify commit rangesJohannes Schindelin via GitGitGadget, Jan 27, 2021
  37. 2/3 range-diff/format-patch: handle commit ranges other than A..BJohannes Schindelin via GitGitGadget, Jan 27, 2021
  38. 1/3 range-diff/format-patch: refactor check for commit rangeJohannes Schindelin via GitGitGadget, Jan 27, 2021
  39. 0/3 Range diff with ranges lacking dotdotJohannes Schindelin via GitGitGadget, Feb 4, 2021
  40. 1/3 range-diff/format-patch: refactor check for commit rangeJohannes Schindelin via GitGitGadget, Feb 4, 2021
  41. Junio C HamanoFeb 4, 2021
  42. Johannes SchindelinFeb 4, 2021
  43. 2/3 range-diff/format-patch: handle commit ranges other than A..BJohannes Schindelin via GitGitGadget, Feb 4, 2021
  44. Junio C HamanoFeb 4, 2021
  45. Johannes SchindelinFeb 4, 2021
  46. Junio C HamanoFeb 4, 2021
  47. Johannes SchindelinFeb 4, 2021
  48. 3/3 range-diff(docs): explain how to specify commit rangesJohannes Schindelin via GitGitGadget, Feb 4, 2021
  49. Junio C HamanoFeb 4, 2021
  50. Johannes SchindelinFeb 4, 2021
  51. Junio C HamanoFeb 4, 2021
  52. 0/3 Range diff with ranges lacking dotdotJohannes Schindelin via GitGitGadget, Feb 4, 2021
  53. 3/3 range-diff(docs): explain how to specify commit rangesJohannes Schindelin via GitGitGadget, Feb 4, 2021
  54. 1/3 range-diff/format-patch: refactor check for commit rangeJohannes Schindelin via GitGitGadget, Feb 4, 2021
  55. 2/3 range-diff/format-patch: handle commit ranges other than A..BJohannes Schindelin via GitGitGadget, Feb 4, 2021
  56. Junio C HamanoFeb 5, 2021
  57. Junio C HamanoFeb 5, 2021
  58. Johannes SchindelinFeb 5, 2021
  59. 0/3 Range diff with ranges lacking dotdotJohannes Schindelin via GitGitGadget, Feb 5, 2021
  60. 2/3 range-diff/format-patch: handle commit ranges other than A..BJohannes Schindelin via GitGitGadget, Feb 5, 2021
  61. 3/3 range-diff(docs): explain how to specify commit rangesJohannes Schindelin via GitGitGadget, Feb 5, 2021
  62. 1/3 range-diff/format-patch: refactor check for commit rangeJohannes Schindelin via GitGitGadget, Feb 5, 2021
  63. Johannes SchindelinFeb 6, 2021

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.