The 25/08/09, Junio C Hamano wrote:
> 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.
Ok, thank you.
> 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.
Could find it at the end of the mails.
> > #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?
It is for the warning() call
warning("scissors line found, will skip text above");
I've added. That said, moving this declaration to builtin.h could be a
good idea. Hint?
> > @@ -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.
Good eyes.
> > + /*
> > + * 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.
Yes. Instead of looking for dashes alone, I will give a try to something
like
if (!scissors_dashes_seen)
mark_start = i;
if (i + 1 < len) {
if (!memcmp(buf + i, ">8", 2) || !memcmp(buf + i, "8<", 2))) {
scissors_dashes_seen |= 02;
i++;
mark_end = i;
continue;
else if (!memcmp(buf + i "--", 2) {
scissors_dashes_seen |= 04;
i++;
mark_end = i;
continue;
}
}
if (i + 2 < len)
if (!memcmp(buf + i + 1, "- -", 3) {
scissors_dashes_seen |= 04;
i += 2;
mark_end = i;
continue;
}
if (buf[i] == '-') {
mark_end = i;
scissors_dashes_seen |= 01;
continue;
}
break;
}
if (scissors_dashes_seen == 07) {
...
> it does not allow
>
> "-- 8< -- please cut here -- 8< -- --"
Actually, I believe this one should really not be a scissors line. If we
accept some random dashes around markers it will break the definition of
the mark itself.
As I said, I'd rather rules easy to define over others because if the
end-user scissors line doesn't work, he can refer to the documentation...
> nor
>
> "-- 8< -- -- please cut here -- -- 8< --"
>
> nor
>
> "-- 8< -- -- please cut here -- -- >8 --"
...and symmetrical markers make sense to the user. Will add this.
> > + 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)) {
> ...
> }
Does the compilers (or a standard) assure that the members are evaluated
in the left-right order?
Otherwise, we may call is_scissors_line() where not needed.
--
Nicolas Sebrecht
--
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