All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Andreas Ericsson <ae@op5.se>,
	Christian Couder <chriscool@tuxfamily.org>,
	git@vger.kernel.org, Avery Pennarun <apenwarr@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Jonathan Nieder <jrnieder@gmail.com>, Max Horn <max@quendi.de>
Subject: Re: [PATCH 00/86] replace prefixcmp() with has_prefix()
Date: Tue, 12 Nov 2013 08:53:45 -0800	[thread overview]
Message-ID: <xmqqy54timsm.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20131112083240.GA1684@sigill.intra.peff.net> (Jeff King's message of "Tue, 12 Nov 2013 03:32:40 -0500")

Jeff King <peff@peff.net> writes:

> I am ambivalent on the code churn, but if we do apply it, we should
> probably leave off the final patch (dropping prefixcmp) for a cycle to
> let topics in flight catch up to the change. Just diffing "master" and
> "next", I see some new uses of prefixcmp which will need adjusted, along
> with spots where the patches themselves will cause textual conflicts.

Yes, I did that check too (but between 'maint' and 'pu'). I think it
is a good idea to stop using whatever_cmp() name for things that are
not *cmp() functions in the longer term, but smooth migration is a
bit tricky (but not as tricky as end-user visible transitions).

Even though we already added has_suffix() for tail matches, it is
not too late to rethink, as it is not in 'master' yet.

One thing I noticed is that it is probably misnamed, or at least in
a way that invites confusion.  Can people tell which one of these is
correct without looking at existing callsites?

	has_suffix(filename, "txt");
        has_suffix(filename, ".txt");

The semantics of the function we have is the latter and is better
called endswith(), I suspect.  And the corresponding function to
check for head matches should probably be called beginswith().

  reply	other threads:[~2013-11-12 16:53 UTC|newest]

Thread overview: 100+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-11-09  7:05 [PATCH 00/86] replace prefixcmp() with has_prefix() Christian Couder
2013-11-09  7:05 ` [PATCH 01/86] strbuf: add has_prefix() to be used instead of prefixcmp() Christian Couder
2013-11-09  7:05 ` [PATCH 02/86] diff: replace prefixcmd() with has_prefix() Christian Couder
2013-11-09  7:05 ` [PATCH 03/86] fast-import: " Christian Couder
2013-11-09  7:05 ` [PATCH 04/86] remote*: " Christian Couder
2013-11-09  7:05 ` [PATCH 05/86] daemon: " Christian Couder
2013-11-09  7:05 ` [PATCH 06/86] pretty: " Christian Couder
2013-11-09  7:06 ` [PATCH 07/86] revision: " Christian Couder
2013-11-09  7:06 ` [PATCH 08/86] transport*: " Christian Couder
2013-11-09  7:06 ` [PATCH 09/86] config: " Christian Couder
2013-11-09  7:06 ` [PATCH 10/86] sha1_name: " Christian Couder
2013-11-09  7:06 ` [PATCH 11/86] wt-status: " Christian Couder
2013-11-09  7:06 ` [PATCH 12/86] upload-pack: " Christian Couder
2013-11-09  7:06 ` [PATCH 13/86] test-line-buffer: " Christian Couder
2013-11-09  7:06 ` [PATCH 14/86] parse-options: " Christian Couder
2013-11-09  7:06 ` [PATCH 15/86] fetch-pack: " Christian Couder
2013-11-09  7:06 ` [PATCH 16/86] git: " Christian Couder
2013-11-09  7:06 ` [PATCH 17/86] tag: " Christian Couder
2013-11-09  7:06 ` [PATCH 18/86] sequencer: " Christian Couder
2013-11-09  7:06 ` [PATCH 19/86] commit: " Christian Couder
2013-11-09  7:06 ` [PATCH 20/86] http: " Christian Couder
2013-11-09  7:06 ` [PATCH 21/86] imap-send: " Christian Couder
2013-11-09  7:06 ` [PATCH 22/86] help: " Christian Couder
2013-11-09  7:06 ` [PATCH 23/86] log-tree: " Christian Couder
2013-11-09  7:06 ` [PATCH 24/86] merge-recursive: " Christian Couder
2013-11-09  7:06 ` [PATCH 25/86] notes: " Christian Couder
2013-11-09  7:06 ` [PATCH 26/86] refs: " Christian Couder
2013-11-09  7:06 ` [PATCH 27/86] setup: " Christian Couder
2013-11-09  7:06 ` [PATCH 28/86] bisect: " Christian Couder
2013-11-09  7:06 ` [PATCH 29/86] branch: " Christian Couder
2013-11-09  7:06 ` [PATCH 30/86] http-push: " Christian Couder
2013-11-09  7:06 ` [PATCH 31/86] send-pack: " Christian Couder
2013-11-09  7:06 ` [PATCH 32/86] http-backend: " Christian Couder
2013-11-09  7:06 ` [PATCH 33/86] notes-utils: " Christian Couder
2013-11-09  7:06 ` [PATCH 34/86] pkt-line: " Christian Couder
2013-11-09  7:06 ` [PATCH 35/86] alias: " Christian Couder
2013-11-09  7:06 ` [PATCH 36/86] attr: " Christian Couder
2013-11-09  7:06 ` [PATCH 37/86] connect: " Christian Couder
2013-11-09  7:06 ` [PATCH 38/86] pager: " Christian Couder
2013-11-09  7:06 ` [PATCH 39/86] convert: " Christian Couder
2013-11-09  7:06 ` [PATCH 40/86] environment: " Christian Couder
2013-11-09  7:06 ` [PATCH 41/86] shell: " Christian Couder
2013-11-09  7:06 ` [PATCH 42/86] pathspec: " Christian Couder
2013-11-09  7:06 ` [PATCH 43/86] submodule: " Christian Couder
2013-11-09  7:06 ` [PATCH 44/86] test-string-list: " Christian Couder
2013-11-09  7:06 ` [PATCH 45/86] builtin/apply: " Christian Couder
2013-11-09  7:06 ` [PATCH 46/86] builtin/archive: " Christian Couder
2013-11-09  7:06 ` [PATCH 47/86] builtin/branch: " Christian Couder
2013-11-09  7:06 ` [PATCH 48/86] builtin/checkout: " Christian Couder
2013-11-09  7:06 ` [PATCH 49/86] builtin/clean: " Christian Couder
2013-11-09  7:06 ` [PATCH 50/86] builtin/clone: " Christian Couder
2013-11-09  7:06 ` [PATCH 51/86] builtin/column: " Christian Couder
2013-11-09  7:06 ` [PATCH 52/86] builtin/commit: " Christian Couder
2013-11-09  7:06 ` [PATCH 53/86] builtin/describe: " Christian Couder
2013-11-09  7:06 ` [PATCH 54/86] builtin/fast-export: " Christian Couder
2013-11-09  7:06 ` [PATCH 55/86] builtin/fetch-pack: " Christian Couder
2013-11-09  7:06 ` [PATCH 56/86] builtin/fetch: " Christian Couder
2013-11-09  7:06 ` [PATCH 57/86] builtin/fmt-merge-msg: " Christian Couder
2013-11-09  7:06 ` [PATCH 58/86] builtin/for-each-ref: " Christian Couder
2013-11-09  7:06 ` [PATCH 59/86] builtin/fsck: " Christian Couder
2013-11-09  7:06 ` [PATCH 60/86] builtin/help: " Christian Couder
2013-11-09  7:06 ` [PATCH 61/86] builtin/index-pack: " Christian Couder
2013-11-09  7:06 ` [PATCH 62/86] builtin/init-db: " Christian Couder
2013-11-09  7:06 ` [PATCH 63/86] builtin/log: " Christian Couder
2013-11-09  7:06 ` [PATCH 64/86] builtin/ls-remote: " Christian Couder
2013-11-09  7:06 ` [PATCH 65/86] builtin/mailinfo: " Christian Couder
2013-11-09  7:06 ` [PATCH 66/86] builtin/merge-recursive: " Christian Couder
2013-11-09  7:07 ` [PATCH 67/86] builtin/merge: " Christian Couder
2013-11-09  7:07 ` [PATCH 68/86] builtin/name-rev: " Christian Couder
2013-11-09  7:07 ` [PATCH 69/86] builtin/notes: " Christian Couder
2013-11-09  7:07 ` [PATCH 70/86] builtin/pack-objects: " Christian Couder
2013-11-09  7:07 ` [PATCH 71/86] builtin/prune: " Christian Couder
2013-11-09  7:07 ` [PATCH 72/86] builtin/receive-pack: " Christian Couder
2013-11-09  7:07 ` [PATCH 73/86] builtin/reflog: " Christian Couder
2013-11-09  7:07 ` [PATCH 74/86] builtin/remote: " Christian Couder
2013-11-09  7:07 ` [PATCH 75/86] builtin/rev-parse: " Christian Couder
2013-11-09  7:07 ` [PATCH 76/86] builtin/send-pack: " Christian Couder
2013-11-09  7:07 ` [PATCH 77/86] builtin/shortlog: " Christian Couder
2013-11-09  7:07 ` [PATCH 78/86] builtin/show-branch: " Christian Couder
2013-11-09  7:07 ` [PATCH 79/86] builtin/show-ref: " Christian Couder
2013-11-09  7:07 ` [PATCH 80/86] builtin/symbolic-ref: " Christian Couder
2013-11-09  7:07 ` [PATCH 81/86] builtin/tag: " Christian Couder
2013-11-09  7:07 ` [PATCH 82/86] builtin/tar-tree: " Christian Couder
2013-11-09  7:07 ` [PATCH 83/86] builtin/unpack-objects: " Christian Couder
2013-11-09  7:07 ` [PATCH 84/86] builtin/update-ref: " Christian Couder
2013-11-09  7:07 ` [PATCH 85/86] builtin/upload-archive: " Christian Couder
2013-11-09  7:07 ` [PATCH 86/86] strbuf: remove prefixcmp() as it has been replaced " Christian Couder
2013-11-09 14:24 ` [PATCH 00/86] replace prefixcmp() " Thomas Rast
2013-11-12  6:16   ` Christian Couder
2013-11-11 16:09 ` Andreas Ericsson
2013-11-12  8:32   ` Jeff King
2013-11-12 16:53     ` Junio C Hamano [this message]
2013-11-12 20:14       ` Jeff King
2013-11-12 20:43       ` Christian Couder
2013-11-12 21:26         ` Junio C Hamano
2013-11-13  6:47           ` Christian Couder
2013-11-13  7:17             ` Jeff King
2013-11-17  8:52               ` Christian Couder
2013-11-18 10:42                 ` Christian Couder
2013-11-19 21:42                   ` Junio C Hamano

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

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

  git send-email \
    --in-reply-to=xmqqy54timsm.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=ae@op5.se \
    --cc=apenwarr@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=max@quendi.de \
    --cc=peff@peff.net \
    /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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.