From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff King Subject: Re: [PATCH] checkout: add --progress option Date: Sat, 31 Oct 2015 16:07:02 -0400 Message-ID: <20151031200702.GA4115@sigill.intra.peff.net> References: <1446168186-4730-1-git-send-email-eantoranz@gmail.com> <20151030193151.GB5336@sigill.intra.peff.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Cc: Junio C Hamano , Eric Sunshine , Git List To: Edmundo Carmona Antoranz X-From: git-owner@vger.kernel.org Sat Oct 31 21:07:51 2015 Return-path: Envelope-to: gcvg-git-2@plane.gmane.org Received: from vger.kernel.org ([209.132.180.67]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1ZscRJ-00041u-Vz for gcvg-git-2@plane.gmane.org; Sat, 31 Oct 2015 21:07:50 +0100 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751649AbbJaUHh (ORCPT ); Sat, 31 Oct 2015 16:07:37 -0400 Received: from cloud.peff.net ([50.56.180.127]:50984 "HELO cloud.peff.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S1751620AbbJaUHF (ORCPT ); Sat, 31 Oct 2015 16:07:05 -0400 Received: (qmail 23738 invoked by uid 102); 31 Oct 2015 20:07:05 -0000 Received: from Unknown (HELO peff.net) (10.0.1.1) by cloud.peff.net (qpsmtpd/0.84) with SMTP; Sat, 31 Oct 2015 15:07:05 -0500 Received: (qmail 575 invoked by uid 107); 31 Oct 2015 20:07:30 -0000 Received: from sigill.intra.peff.net (HELO sigill.intra.peff.net) (10.0.0.7) by peff.net (qpsmtpd/0.84) with SMTP; Sat, 31 Oct 2015 16:07:30 -0400 Received: by sigill.intra.peff.net (sSMTP sendmail emulation); Sat, 31 Oct 2015 16:07:02 -0400 Content-Disposition: inline In-Reply-To: Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Archived-At: On Sat, Oct 31, 2015 at 12:14:39PM -0600, Edmundo Carmona Antoranz wrote: > On Sat, Oct 31, 2015 at 11:42 AM, Edmundo Carmona Antoranz > wrote: > > On Sat, Oct 31, 2015 at 11:37 AM, Junio C Hamano wrote: > >> I do find what Peff showed us a lot easier to follow. > >> > >> if (opts.show_progress < 0) { > >> if (opts.quiet) > >> opts.show_progress = 0; > >> else > >> opts.show_progress = isatty(2); > >> } > >> > > > > Ok.... let me rewrite it that way. Other than that, the other things are ok? > > In Peff's implementation I think he uses -1 as --no-progress, 1 as > --progress and 0 as undefined, right? I didn't mean to, though I don't promise I didn't send something buggy. It looks right to me, though: if (opts.show_progress < 0) { /* if the user didn't say... */ if (opts.quiet) opts.show_progress = 0; /* quiet means "no progress" */ else opts.show_progress = isatty(2); /* returns 0/1 bool */ } > In my implementation I'm using -1 as undefined and 0 as --no-progress. > What would be the standard approach? That's standard. And "1" is "--progress". > From what I can see on > parse_options's behavior, if you select --no-progress, the variable > ends up with a 0, which makes me think I'm using the right approach. > > End result with my assumptions would be: > > if (opts.show_progress) { > /* user selected --progress or didn't specify */ > if (opts.quiet) { > opts.show_progress = 0; > } else if (opts.show_progress < 0) { > opts.show_progress = isatty(2); > } > } The difference between mine and yours is that in mine, "--progress" trumps "--quiet", whereas it is the other way around in yours. I don't know if it is a huge deal, but mine makes more sense to me (because "--progress" is more specific than "--quiet", which might silence other messages, too). -Peff