From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS31976 209.132.180.0/23 X-Spam-Status: No, score=-3.8 required=3.0 tests=AWL,BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,RCVD_IN_DNSWL_HI,RP_MATCHES_RCVD shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by dcvr.yhbt.net (Postfix) with ESMTP id 7660220188 for ; Fri, 12 May 2017 08:40:35 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756470AbdELIkc (ORCPT ); Fri, 12 May 2017 04:40:32 -0400 Received: from cloud.peff.net ([104.130.231.41]:50192 "EHLO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755605AbdELIkb (ORCPT ); Fri, 12 May 2017 04:40:31 -0400 Received: (qmail 1664 invoked by uid 109); 12 May 2017 08:40:30 -0000 Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.84) with SMTP; Fri, 12 May 2017 08:40:30 +0000 Received: (qmail 22223 invoked by uid 111); 12 May 2017 08:41:02 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.84) with SMTP; Fri, 12 May 2017 04:41:02 -0400 Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Fri, 12 May 2017 04:40:28 -0400 Date: Fri, 12 May 2017 04:40:28 -0400 From: Jeff King To: Brian Malehorn Cc: git@vger.kernel.org Subject: Re: [PATCH 2/3] commit.c: add is_scissors_line Message-ID: <20170512084028.f6vv4uz6of6ofadu@sigill.intra.peff.net> References: <20170512050347.30765-1-bmalehorn@gmail.com> <20170512050347.30765-3-bmalehorn@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170512050347.30765-3-bmalehorn@gmail.com> Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org On Thu, May 11, 2017 at 10:03:46PM -0700, Brian Malehorn wrote: > Move is_scissors_line to commit.c and expose it through commit.h. > This is needed in commit.c, and mailinfo.c shouldn't really own it. It was fine for mailinfo to own it until now, since it was the only user. :) I think there are some unsaid bits in your rationale. Perhaps something like: Subject: mailinfo: make is_scissors_line() public We parse scissors lines only when looking at emails, and thus the parsing function has always lived in mailinfo.c. However, we also generate scissors lines as part of "git commit -v". We should be able to reuse the same function to parse them. Once public, it doesn't really belong in mailinfo.c anymore. A better place is commit.c because...[I had trouble filling this in; it's really _not_ about commits in particular, but rather about the in-editor representation of the commit message used by git-commit]. Thinking on it more, though...are the scissors lines generated by "-v" really the same as the ones parsed by mailinfo? In the case of mailinfo, we are parsing an externally generated string according to a heuristic microformat. But with "commit -v", we are the ones who wrote the cut line in the first place. Shouldn't we be a bit more picky and make sure we parse the exact same cut line? And indeed, we _do_ actually have to parse these cut lines already, when we read the commit message back in. The code is in wt_status_truncate_message_at_cut_line(), and it does do an exact match. We should probably be using that rather than making the mailinfo is_scissors_line() public. -Peff