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

Re: [PATCH v2 3/3] credential: handle `credential.<partial-URL>.<key>` again

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Apr 24, 2020, 11:34 UTC
Message-ID
<nycvar.QRO.7.76.6.2004241331180.18039@tvgsbejvaqbjf.bet>
In-Reply-To
<20200424003926.GC20669@Carlos-MBP>
Hi Carlo,
On Thu, 23 Apr 2020, Carlo Marcelo Arenas Belón wrote:
Show 28 quoted lines
> On Fri, Apr 24, 2020 at 02:16:45AM +0200, Johannes Schindelin wrote:
> > On Thu, 23 Apr 2020, Carlo Marcelo Arenas Belón wrote:
> > > On Thu, Apr 23, 2020 at 11:43:17PM +0000, Johannes Schindelin via GitGitGadget wrote:
> > > > diff --git a/credential.c b/credential.c
> > > > index 52965a5122c..3505f6356d8 100644
> > > > --- a/credential.c
> > > > +++ b/credential.c
> > > > @@ -53,7 +53,13 @@ static int credential_config_callback(const char *var, const char *value,
> > > >  		char *url = xmemdupz(key, dot - key);
> > > >  		int matched;
> > > >
> > > > -		credential_from_url(&want, url);
> > > > +		if (credential_from_url_gently(&want, url, 1, 0) < 0) {
> > >
> > > definitely not worth a reroll, but just wondering if would make sense to call
> > > credential_from_url_gently(!quiet) here, just for consistency?
> >
> > We don't have any `quiet` variable here.
>
> my bad, should had been more explicit as it is confusing with all those
> booleans at the end without a flags parameter.
>
> I mean the call should be instead :
>
>   if (credential_from_url_gently(&want, url, 1, 1) < 0) {
>
> since we want to warn if the configuration is not supported just like is
> done after this check.

Since Junio expressed support for Jonathan's idea of using separate functions that wrap one helper function, I went with that instead.

Show 10 quoted lines
> > > other than that this series is looking great, under the assumption that there
> > > is going to be some more followup with non essential changes.
> >
> > I am sure I don't follow. What non-essential changes are you assuming will
> > follow up?
>
> the ones that were discussed with Jonathan in a differen thread :
>
> * using a flags parameter instead of two ints
> * whatever will be needed so it also works in master and maint

Oy, I am still under water just trying to get the MinGit for Windows backports updated so that users can upgrade instead of complaining on bug trackers and on Twitter...

But yes, that will be the next step. Now that I have a comprehensive test case, it should be relatively easy.

Show 10 quoted lines
> > > will chip in with an test helper for that series so we can hopefully keep our
> > > sanity next time someone touches that function again.
> >
> > Are the tests I added to t0300 not enough? Or do you think it will need a
> > native test helper that is included in `t/helper/test-tool` and exercised
> > in the test suite somehow?
>
> I think they are enough, it is only that is easier to quickly iterate with
> possible bad input with a helper. which is what I was offering for the next
> thread since its need is orthogonal to what you are trying to accomplish.

Usually I would agree with you. It's quicker to iterate, and a ton faster to run (especially on Windows).

In this instance, I am going for a regression test case rather than a unit test, though, because I really need to ensure that what end users are trying to do on their machines will work (and continue to work).

Ciao, Dscho

Previous: Carlo Marcelo Arenas BelónNext: Junio C Hamano
Message 30 of 48 in “credential: handle partial URLs in config settings again”
  1. 0/3 credential: handle partial URLs in config settings againJohannes Schindelin via GitGitGadget, Apr 22, 2020
  2. 1/3 credential: fix grammarJohannes Schindelin via GitGitGadget, Apr 22, 2020
  3. Jonathan NiederApr 22, 2020
  4. 3/3 credential: handle `credential.<partial-URL>.<key>` againJohannes Schindelin via GitGitGadget, Apr 22, 2020
  5. Jonathan NiederApr 22, 2020
  6. Johannes SchindelinApr 23, 2020
  7. 2/3 credential: teach `credential_from_url()` a non-strict modeJohannes Schindelin via GitGitGadget, Apr 22, 2020
  8. Junio C HamanoApr 22, 2020
  9. Johannes SchindelinApr 22, 2020
  10. Junio C HamanoApr 22, 2020
  11. Jonathan NiederApr 22, 2020
  12. Carlo ArenasApr 23, 2020
  13. Johannes SchindelinApr 23, 2020
  14. Carlo Marcelo Arenas BelónApr 23, 2020
  15. Johannes SchindelinApr 23, 2020
  16. Junio C HamanoApr 23, 2020
  17. Junio C HamanoApr 23, 2020
  18. Johannes SchindelinApr 23, 2020
  19. Johannes SchindelinApr 23, 2020
  20. Jeff KingApr 22, 2020
  21. Johannes SchindelinApr 22, 2020
  22. Jonathan NiederApr 22, 2020
  23. Johannes SchindelinApr 23, 2020
  24. 0/3 credential: handle partial URLs in config settings againJohannes Schindelin via GitGitGadget, Apr 23, 2020
  25. 2/3 credential: optionally allow partial URLs in credential_from_url_gently()Johannes Schindelin via GitGitGadget, Apr 23, 2020
  26. 3/3 credential: handle `credential.<partial-URL>.<key>` againJohannes Schindelin via GitGitGadget, Apr 23, 2020
  27. Carlo Marcelo Arenas BelónApr 24, 2020
  28. Johannes SchindelinApr 24, 2020
  29. Carlo Marcelo Arenas BelónApr 24, 2020
  30. Johannes SchindelinApr 24, 2020
  31. Junio C HamanoApr 24, 2020
  32. Junio C HamanoApr 24, 2020
  33. Johannes SchindelinApr 24, 2020
  34. 1/3 credential: fix grammarJohannes Schindelin via GitGitGadget, Apr 23, 2020
  35. Carlo Marcelo Arenas BelónApr 24, 2020
  36. 0/3 credential: handle partial URLs in config settings againJohannes Schindelin via GitGitGadget, Apr 24, 2020
  37. 1/3 credential: fix grammarJohannes Schindelin via GitGitGadget, Apr 24, 2020
  38. 3/3 credential: handle `credential.<partial-URL>.<key>` againJohannes Schindelin via GitGitGadget, Apr 24, 2020
  39. Carlo Marcelo Arenas BelónApr 24, 2020
  40. Johannes SchindelinApr 25, 2020
  41. Junio C HamanoApr 24, 2020
  42. Johannes SchindelinApr 24, 2020
  43. Junio C HamanoApr 28, 2020
  44. Johannes SchindelinApr 29, 2020
  45. Junio C HamanoApr 29, 2020
  46. Johannes SchindelinMay 1, 2020
  47. Junio C HamanoMay 1, 2020
  48. 2/3 credential: optionally allow partial URLs in credential_from_url_gently()Johannes Schindelin via GitGitGadget, Apr 24, 2020

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.