X-Authentication-Warning: delorie.com: mail set sender to geda-user-bounces using -f
X-Recipient: geda-user AT delorie DOT com
Date: Sat, 24 Oct 2020 01:55:24 +0200 (CEST)
From: Roland Lutz <rlutz AT hedmen DOT org>
To: "Glenn (glimrick AT epilitimus DOT com) [via geda-user AT delorie DOT com]" <geda-user AT delorie DOT com>
Subject: Re: [geda-user] submitted a new patch
In-Reply-To: <7df6cee0-b96c-1753-29a6-58026eeb991b@epilitimus.com>
Message-ID: <alpine.DEB.2.21.2010240119430.5003@nimbus>
References: <14f9e862-8ee0-4432-23b6-06e94215baa4 AT epilitimus DOT com> <alpine DOT DEB DOT 2 DOT 21 DOT 2010120958150 DOT 2535 AT nimbus> <32bfe083-3604-b747-030a-48a13e2b1074 AT epilitimus DOT com> <alpine DOT DEB DOT 2 DOT 21 DOT 2010122312420 DOT 8245 AT nimbus> <7c133ba2-5b09-91f3-808f-9f444c625278 AT epilitimus DOT com>
 <alpine DOT DEB DOT 2 DOT 21 DOT 2010151343310 DOT 1527 AT nimbus> <aa7c0456-a606-be86-58c4-b8352cc66127 AT epilitimus DOT com> <alpine DOT DEB DOT 2 DOT 21 DOT 2010231526280 DOT 12318 AT nimbus> <7df6cee0-b96c-1753-29a6-58026eeb991b AT epilitimus DOT com>
User-Agent: Alpine 2.21 (DEB 202 2017-01-01)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="8323329-459796504-1603497324=:5003"
Reply-To: geda-user AT delorie DOT com
Errors-To: nobody AT delorie DOT com
X-Mailing-List: geda-user AT delorie DOT com
X-Unsubscribes-To: listserv AT delorie DOT com
Precedence: bulk

  This message is in MIME format.  The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--8323329-459796504-1603497324=:5003
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8BIT

On Fri, 23 Oct 2020, Glenn (glimrick AT epilitimus DOT com) [via 
geda-user AT delorie DOT com] wrote:
> Okay so the newline was there because of the debug message you removed.
> It pushed the "Checking for title box" message to the next line.

Oh, I see.  So debug-spew requires a newline at the end of the string if a 
newline is supposed to be printed; it's just missing from two messages in 
this function.  This hasn't been noticed so far because the next message 
starts with a newline.

This is actually one of the cases where it makes sense to submit a second 
patch fixing the newlines.  It's normally preferred to not change small 
things like whitespace unless necessary; however, in this case, it's 
clearly an oversight (see commit 86f100d).

> I think the comment you are referring to was actually my rewrite of the
> comment to include the fact that a specific title was permitted.  So
> here I would argue that the change was relevant, but actually to the
> previous patch that added the spice-title rather than this one.

Extending the top comment is totally fine (and good practice).

I was referring to the removal of the comment ";; If the schematic 
contains a spice-title device" a few lines below.  Sure, it's a kind of 
obvious thing to state, and maybe it was bad practice to place the comment 
in the first place; but once it's there, it should only be removed if 
there's reason to do so--like the piece of code changed, the comment isn't 
accurate any more, or you are the original author of the comment and feel 
like it.  (Don't spend too much thought about this, though.)

Roland

--8323329-459796504-1603497324=:5003--