Re: [PATCH 2/3] stripspace: Implement --count-lines option

2 messages, 2 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 2/3] stripspace: Implement --count-lines option

From: Matthieu Moy <hidden>
Date: 2016-06-15 23:06:53

Tobias Klauser [off-list ref] writes:
+ * comments are considered contents to be removed or not. Returns the
+ * number of lines in the resulting buffer.
We write comments at imperative tone, hence "Return", not "Returns".

Other than that, I agree with Junio:

* A preparatory patch introducing parse-options would make the actual
  patch much easier to review.

* Just running strbuf_stripspace and counting the number of lines in the
  result is much simpler. We use stripspace only on user-provided input
  which are never really big so maintainability is more important than
  performance.

Cheers,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH 2/3] stripspace: Implement --count-lines option

From: Tobias Klauser <tklauser@distanz.ch>
Date: 2016-06-15 23:06:53

On 2015-10-15 at 21:21:53 +0200, Matthieu Moy [off-list ref] wrote:
Tobias Klauser [off-list ref] writes:
quoted
+ * comments are considered contents to be removed or not. Returns the
+ * number of lines in the resulting buffer.
We write comments at imperative tone, hence "Return", not "Returns".
The other comments in strbuf.h used "Returns", so I went for it for
consistency reasons. But the comment will be obsolete anyhow, as
strbuf_stripspace() will not be changed and the functionality
implemented in cmd_stripspace() as Junio suggested.
Other than that, I agree with Junio:

* A preparatory patch introducing parse-options would make the actual
  patch much easier to review.
Will do.
* Just running strbuf_stripspace and counting the number of lines in the
  result is much simpler. We use stripspace only on user-provided input
  which are never really big so maintainability is more important than
  performance.
Ditto.

Thanks
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help