Nicolas Sebrecht <nicolas.s.dev@xxxxxx> writes:
>> I think we have bikeshedded long enough, so I won't be touching this code
>> any further only to change the definition of what a scissors mark looks
>> like,
>
> I'm not sure I understand. Are you still open to a patch touching this code
> /too/?
What I meant was that I would not want to spend any more of _my_ time on
the definition of the scissors for now. That means spending or wasting
time on improving the 'pu' patch myself, or looking at others patch to
find flaws in them.
Of course, as the maintainer, I would need to look at proposals to improve
or fix bugs in the code before the series hits the master, but I would
give zero priority to the patches that change the definition at least for
now to give myself time to work on more useful things.
I think --ignore-scissors is a good thing to add, regardless of what the
definition of scissors should be. So your patch should definitely be
separated into two parts.
> diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
> index 7e09b51..92319f6 100644
> --- a/builtin-mailinfo.c
> +++ b/builtin-mailinfo.c
> @@ -6,6 +6,7 @@
> #include "builtin.h"
> #include "utf8.h"
> #include "strbuf.h"
> +#include "git-compat-util.h"
Inclusion of builtin.h is designed to be enough. What do you need this
for?
> static FILE *cmitmsg, *patchfile, *fin, *fout;
>
> @@ -25,6 +26,7 @@ static enum {
> static struct strbuf charset = STRBUF_INIT;
> static int patch_lines;
> static struct strbuf **p_hdr_data, **s_hdr_data;
> +static int ignore_scissors = 0;
Don't initialize a static to 0.
> @@ -715,51 +717,63 @@ static inline int patchbreak(const struct strbuf *line)
> if (isspace(buf[i])) {
> + if (scissors_dashes_seen)
> + mark_end = i;
I think you do not want this part, and then you won't have to trim
trailing whitespaces from mark_end later.
> + /*
> + * The mark is 8 charaters long and contains at least one dash and
> + * either a ">8" or "<8". Check if the last mark in the line
> + * matches the first mark found without worrying about what could
> + * be between them. Only one mark in the whole line is permitted.
> + */
This definition makes "- 8<" a scissors.
Even though
"-- 8< -- please cut here -- -- 8< --"
is allowed, so is
"-- 8< -- -- please cut here -- 8< -- --"
it does not allow
"-- 8< -- please cut here -- 8< -- --"
nor
"-- 8< -- -- please cut here -- -- 8< --"
nor
"-- 8< -- -- please cut here -- -- >8 --"
Oh, did I say I won't waste my time on the definition? I should have just
discarded this hunk ;-)
> @@ -782,22 +796,25 @@ static int handle_commit_msg(struct strbuf *line)
> if (metainfo_charset)
> convert_to_utf8(line, charset.buf);
>
> - if (is_scissors_line(line)) {
> - int i;
> - rewind(cmitmsg);
> - ftruncate(fileno(cmitmsg), 0);
> - still_looking = 1;
> + if (!ignore_scissors) {
> + if (is_scissors_line(line)) {
> + warning("scissors line found, will skip text above");
> ...
> + return 0;
Don't re-indent like this. Just do:
if (!ignore_scissors && is_scissors_line(line)) {
...
}
> - # -s, -u, -k, --whitespace, -3, -C, -q and -p flags are kept
> + # Following flags are kept
We seem to have lost the description of what the "Following" are.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html