git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Matthew DeVore <matvore@comcast.net>
Cc: Emily Shaffer <emilyshaffer@google.com>,
	Matthew DeVore <matvore@google.com>,
	jonathantanmy@google.com, jrn@google.com, git@vger.kernel.org,
	dstolee@microsoft.com, jeffhost@microsoft.com,
	jrnieder@gmail.com, pclouds@gmail.com
Subject: Re: [PATCH v1 3/5] list-objects-filter: implement composite filters
Date: Tue, 4 Jun 2019 19:14:18 -0400	[thread overview]
Message-ID: <20190604231418.GA12501@sigill.intra.peff.net> (raw)
In-Reply-To: <20190604225921.GA43275@comcast.net>

On Tue, Jun 04, 2019 at 03:59:21PM -0700, Matthew DeVore wrote:

> > I think you'll find that -Wunused-function complains, though, if nobody
> > is calling it. I wasn't sure if what you showed in the interdiff was
> > meant to be final (I had to add a few other variable declarations to
> > make it compile, too).
> 
> Sorry, my last interdiff was a mess because I made a mistake during git rebase
> -i. It was missing a call to has_reserved_char. Below is another diff that
> fixes the problems:

Ah, OK, that makes sense then (and keeping the function is obviously the
right thing to do).

> > That makes some sense, and I agree that readability is a good goal. Do
> > we not need to be escaping colons in other URLs? Or are the strings
> > you're generating not true by-the-book URLs? I'm just wondering if we
> > could take this opportunity to improve the URLs we output elsewhere,
> > too.
> 
> The strings I'm generating are not URLs. Also, in http.c, we have to use : to
> delimit a username and password:
> 
> 	strbuf_addstr_urlencode(&s, proxy_auth.username, 1);
> 	strbuf_addch(&s, ':');
> 	strbuf_addstr_urlencode(&s, proxy_auth.password, 1);
> 
> I think this is dictated by libcurl and is not flexible.

Right, that has to be a real colon because it's syntactically
significant (but a colon in the username _must_ be encoded). That strbuf
function doesn't really understand whole URLs, and it's up to the caller
to assemble the parts.

Anyway, we've veered off of your patch series enough. Yeah, it sounds
like using strbuf's url-encoding is not quite what you want.

-Peff

  reply	other threads:[~2019-06-04 23:14 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-22  0:21 [PATCH v1 0/5] Filter combination Matthew DeVore
2019-05-22  0:21 ` [PATCH v1 1/5] list-objects-filter: refactor into a context struct Matthew DeVore
2019-05-24  0:49   ` Emily Shaffer
2019-05-28 18:48     ` Matthew DeVore
2019-05-28 22:40       ` [PATCH] list-objects-filter: merge filter data structs Matthew DeVore
2019-05-29 19:48         ` Junio C Hamano
2019-05-29 20:57           ` Jeff Hostetler
2019-05-29 23:10             ` Matthew DeVore
2019-05-30  1:56             ` [RFC PATCH v2] " Matthew DeVore
2019-05-30 16:12               ` Junio C Hamano
2019-05-30 18:29                 ` Matthew DeVore
2019-05-30 19:05             ` [PATCH] " Matthew DeVore
2019-05-22  0:21 ` [PATCH v1 2/5] list-objects-filter-options: error is localizeable Matthew DeVore
2019-05-24  0:55   ` Emily Shaffer
2019-05-28 23:01     ` Matthew DeVore
2019-05-22  0:21 ` [PATCH v1 3/5] list-objects-filter: implement composite filters Matthew DeVore
2019-05-24 21:01   ` Jeff Hostetler
2019-05-28 17:59     ` Junio C Hamano
2019-05-29 15:02       ` Matthew DeVore
2019-05-29 21:29         ` Jeff Hostetler
2019-05-29 23:27           ` Matthew DeVore
2019-05-30 14:01             ` Jeff Hostetler
2019-05-31 20:53               ` Matthew DeVore
2019-06-03 21:04                 ` Jeff Hostetler
2019-06-01  0:11     ` Matthew DeVore
2019-05-28 21:53   ` Emily Shaffer
2019-05-31 20:48     ` Matthew DeVore
2019-05-31 21:10       ` Jeff King
2019-06-01  0:12         ` Matthew DeVore
2019-06-03 12:34           ` Jeff King
2019-06-03 22:22             ` Matthew DeVore
2019-06-04 16:13               ` Jeff King
2019-06-04 17:19                 ` Matthew DeVore
2019-06-04 18:51                   ` Jeff King
2019-06-04 22:59                     ` Matthew DeVore
2019-06-04 23:14                       ` Jeff King [this message]
2019-06-04 23:49                         ` Matthew DeVore
2019-06-09 12:36                           ` Jeff King
2019-05-22  0:21 ` [PATCH v1 4/5] list-objects-filter-options: move error check up Matthew DeVore
2019-05-22  0:21 ` [PATCH v1 5/5] list-objects-filter-options: allow mult. --filter Matthew DeVore
2019-06-06 22:44 ` [PATCH v1 0/5] Filter combination Matthew DeVore

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190604231418.GA12501@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=dstolee@microsoft.com \
    --cc=emilyshaffer@google.com \
    --cc=git@vger.kernel.org \
    --cc=jeffhost@microsoft.com \
    --cc=jonathantanmy@google.com \
    --cc=jrn@google.com \
    --cc=jrnieder@gmail.com \
    --cc=matvore@comcast.net \
    --cc=matvore@google.com \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).