[PATCH] Add -ftabstop=WIDTH

[PATCH] Add -ftabstop=WIDTH

[Hannes Eder]

Make tokenizer aware of tabstops and add the commandline option:

-ftabstop=WIDTH
    Set the distance between tab stops.  This helps sparse report correct
    column numbers in warnings or errors.  If the value is less than 1 or
    greater than 100, the option is ignored.  The default is 8.

Signed-off-by: Hannes Eder <hannes@hanneseder.net>
---
 lib.c      |   31 +++++++++++++++++++++++++++++++
 sparse.1   |    7 +++++++
 token.h    |    1 +
 tokenize.c |    8 ++++++--
 4 files changed, 45 insertions(+), 2 deletions(-)

diff --git a/lib.c b/lib.c
index 059ba3b..80dee3c 100644
--- a/lib.c
+++ b/lib.c
@@ -15,6 +15,8 @@
 #include <string.h>
 #include <unistd.h>
 #include <assert.h>
+#include <limits.h>
+#include <errno.h>
 
 #include <sys/types.h>
 
@@ -511,6 +513,35 @@ static char **handle_switch_f(char *arg, char **next)
 	int flag = 1;
 
 	arg++;
+
+	if (!strncmp (arg, "tabstop=", 8)) {
+		char *end;
+		long val;
+		arg += 8;
+
+		errno = 0;
+		val = strtol(arg, &end, 10);
+
+		if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
+		    || (errno != 0 && val == 0)) {
+			die("invalid argument for the -ftabstop= option: %s", strerror(errno));
+		}
+
+		if (end == arg) {
+			die("missing argument for the -ftabstop= option");
+		}
+
+		if (*end != '\0') {
+			/* further characters after number */
+			/* we just ignore them */
+		}
+
+		if (1 <= val && val <= 100)
+			tabstop_width = val;
+
+		return next;
+	}
+
 	if (!strncmp(arg, "no-", 3)) {
 		flag = 0;
 		arg += 3;
diff --git a/sparse.1 b/sparse.1
index d242dc7..a9e3397 100644
--- a/sparse.1
+++ b/sparse.1
@@ -287,6 +287,13 @@ However, this behavior can lead to subtle errors.
 
 Sparse does not issue these warnings by default.
 .
+.SH OTHER OPTIONS
+.TP
+.B \-ftabstop=WIDTH
+Set the distance between tab stops.  This helps sparse report correct
+column numbers in warnings or errors.  If the value is less than 1 or
+greater than 100, the option is ignored.  The default is 8.
+.
 .SH SEE ALSO
 .BR cgcc (1)
 .
diff --git a/token.h b/token.h
index ba7866d..a15ad84 100644
--- a/token.h
+++ b/token.h
@@ -48,6 +48,7 @@ struct stream {
 
 extern int input_stream_nr;
 extern struct stream *input_streams;
+extern int tabstop_width;
 
 struct ident {
 	struct ident *next;	/* Hash chain of identifiers */
diff --git a/tokenize.c b/tokenize.c
index d154882..83cc75e 100644
--- a/tokenize.c
+++ b/tokenize.c
@@ -25,6 +25,7 @@
 int input_stream_nr = 0;
 struct stream *input_streams;
 static int input_streams_allocated;
+int tabstop_width = 8;
 
 #define BUFSIZE (8192)
 
@@ -232,7 +233,10 @@ repeat:
 		goto repeat;
 	}
 
-	stream->pos++;
+	if (c == '\t')
+		stream->pos = ((stream->pos - 1) / tabstop_width + 1) * tabstop_width;
+	else
+		stream->pos++;
 
 	if (c == '\n') {
 		stream->line++;
@@ -291,7 +295,7 @@ static inline int nextchar(stream_t *stream)
 	if (offset < stream->size) {
 		int c = stream->buffer[offset++];
 		static const char special[256] = {
-			['\r'] = 1, ['\n'] = 1, ['\\'] = 1
+			['\t'] = 1, ['\r'] = 1, ['\n'] = 1, ['\\'] = 1
 		};
 		if (!special[c]) {
 			stream->offset = offset;

Re: [PATCH] Add -ftabstop=WIDTH

[Alexey Zaytsev]

On Wed, Dec 31, 2008 at 15:41, Hannes Eder <hannes@hanneseder.net> wrote:
> Make tokenizer aware of tabstops and add the commandline option:
>
> -ftabstop=WIDTH
>    Set the distance between tab stops.  This helps sparse report correct
>    column numbers in warnings or errors.  If the value is less than 1 or
>    greater than 100, the option is ignored.  The default is 8.

Handling tabs is probably a good idea, but making tabstop an option looks
a bit like overkill. Maybe just hardcode it as 8?

Re: [PATCH] Add -ftabstop=WIDTH

[Christopher Li]

[-- Attachment #1: Type: text/plain, Size: 618 bytes --]

On Wed, Dec 31, 2008 at 4:41 AM, Hannes Eder <hannes@hanneseder.net> wrote:
> Make tokenizer aware of tabstops and add the commandline option:
>
> -ftabstop=WIDTH
>    Set the distance between tab stops.  This helps sparse report correct
>    column numbers in warnings or errors.  If the value is less than 1 or
>    greater than 100, the option is ignored.  The default is 8.
>

I took the liberty to simplify the patch a little bit. See the attachment.

If there is no objections, I am going to apply the combined patch to
my repository:

git://git.kernel.org/pub/scm/devel/sparse/chrisl/sparse.git

Thanks,

Chris

[-- Attachment #2: 0001-Simplify-fstabstop-patch.patch --]
[-- Type: application/octet-stream, Size: 2683 bytes --]

From 247fe1aa9fec8e913b533fff28b1412fc6cbb941 Mon Sep 17 00:00:00 2001
From: Christopher Li <sparse@chrisli.org>
Date: Wed, 31 Dec 2008 12:13:11 -0800
Subject: [PATCH] Simplify -fstabstop patch

Signed-off-by: Christopher Li <sparse@chrisli.org>
---
 lib.c      |   29 +++++++----------------------
 token.h    |    2 +-
 tokenize.c |   10 +++++-----
 3 files changed, 13 insertions(+), 28 deletions(-)

diff --git a/lib.c b/lib.c
index 7bad451..de0b09a 100644
--- a/lib.c
+++ b/lib.c
@@ -532,33 +532,18 @@ static char **handle_switch_f(char *arg, char **next)
 
 	if (!strncmp (arg, "tabstop=", 8)) {
 		char *end;
-		long val;
+		unsigned long val;
 		arg += 8;
 
 		errno = 0;
-		val = strtol(arg, &end, 10);
+		val = strtoul(arg, &end, 10);
 
-		if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN))
-		    || (errno != 0 && val == 0)) {
-			die("invalid argument for the -ftabstop= option: %s", strerror(errno));
-		}
-
-		if (end == arg) {
-			die("missing argument for the -ftabstop= option");
-		}
-
-		if (*end != '\0') {
-			/* further characters after number */
-			/* we just ignore them */
-		}
-
-		if (1 <= val && val <= 100)
-			tabstop_width = val;
-
-		return next;
-	}
+		if (!val || val > 100)
+			die("invalid argument for the -ftabstop= option: %ld", val);
+		printf("tabstop= %ld\n", val);
+		tabstop_width = val;
 
-	if (!strncmp(arg, "no-", 3)) {
+	} else if (!strncmp(arg, "no-", 3)) {
 		flag = 0;
 		arg += 3;
 	}
diff --git a/token.h b/token.h
index df7a80c..4c99887 100644
--- a/token.h
+++ b/token.h
@@ -48,7 +48,7 @@ struct stream {
 
 extern int input_stream_nr;
 extern struct stream *input_streams;
-extern int tabstop_width;
+extern unsigned int tabstop_width;
 
 struct ident {
 	struct ident *next;	/* Hash chain of identifiers */
diff --git a/tokenize.c b/tokenize.c
index 8b3ec2f..e6f9d13 100644
--- a/tokenize.c
+++ b/tokenize.c
@@ -25,7 +25,7 @@
 int input_stream_nr = 0;
 struct stream *input_streams;
 static int input_streams_allocated;
-int tabstop_width = 8;
+unsigned int tabstop_width = 8;
 
 #define BUFSIZE (8192)
 
@@ -207,7 +207,7 @@ static int nextchar_slow(stream_t *stream)
 	int offset = stream->offset;
 	int size = stream->size;
 	int c;
-	int spliced = 0, had_cr, had_backslash, complain;
+	int spliced = 0, had_cr, had_backslash, complain, delta;
 
 restart:
 	had_cr = had_backslash = complain = 0;
@@ -233,10 +233,10 @@ repeat:
 		goto repeat;
 	}
 
+	delta = 1;
 	if (c == '\t')
-		stream->pos = ((stream->pos - 1) / tabstop_width + 1) * tabstop_width;
-	else
-		stream->pos++;
+		delta = tabstop_width - stream->pos % tabstop_width;
+	stream->pos += delta;
 
 	if (c == '\n') {
 		stream->line++;
-- 
1.6.0.6

Re: [PATCH v2] Add -ftabstop=WIDTH

[Hannes Eder]

Make tokenizer aware of tabstops and add the commandline option:

-ftabstop=WIDTH
    Set the distance between tab stops.  This helps sparse report correct
    column numbers in warnings or errors.  If the value is less than 1 or
    greater than 100, the option is ignored.  The default is 8.

With simplifications suggested by Christopher Li.

Signed-off-by: Hannes Eder <hannes@hanneseder.net>
---

On Wed, Dec 31, 2008 at 9:26 PM, Christopher Li <sparse@chrisli.org> wrote:
> I took the liberty to simplify the patch a little bit. See the attachment.

I like the part in tokenizer.c, I even simplified it a bit further, to
use a ternary op, and
do not use a variable.

> If there is no objections, I am going to apply the combined patch to
> my repository:

I slightly modified to option parsing part, to be at least as relaxed
as GNU's CPP, where the option is from (see manpage), and to match the
description in sparse.1.

> With simplifications suggested by Christopher Li.

How should I acknowledge your contribution? Is this Ok?

Or shoul I send a patch relative to your patch?


 lib.c      |   17 ++++++++++++++++-
 sparse.1   |    7 +++++++
 token.h    |    1 +
 tokenize.c |    5 +++--
 4 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/lib.c b/lib.c
index 059ba3b..221e7b8 100644
--- a/lib.c
+++ b/lib.c
@@ -15,6 +15,8 @@
 #include <string.h>
 #include <unistd.h>
 #include <assert.h>
+#include <limits.h>
+#include <errno.h>

 #include <sys/types.h>

@@ -511,7 +513,20 @@ static char **handle_switch_f(char *arg, char **next)
 	int flag = 1;

 	arg++;
-	if (!strncmp(arg, "no-", 3)) {
+
+	if (!strncmp(arg, "tabstop=", 8)) {
+		char *end;
+		unsigned long val;
+		arg += 8;
+
+		if (*arg == '\0')
+			die("error: missing argument to \"-ftabstop=\"");
+
+		/* we silently ignore silly values */
+		val = strtoul(arg, &end, 10);
+		if (*end == '\0' && 1 <= val && val <= 100)
+			tabstop = val;
+	} else if (!strncmp(arg, "no-", 3)) {
 		flag = 0;
 		arg += 3;
 	}
diff --git a/sparse.1 b/sparse.1
index d242dc7..a9e3397 100644
--- a/sparse.1
+++ b/sparse.1
@@ -287,6 +287,13 @@ However, this behavior can lead to subtle errors.

 Sparse does not issue these warnings by default.
 .
+.SH OTHER OPTIONS
+.TP
+.B \-ftabstop=WIDTH
+Set the distance between tab stops.  This helps sparse report correct
+column numbers in warnings or errors.  If the value is less than 1 or
+greater than 100, the option is ignored.  The default is 8.
+.
 .SH SEE ALSO
 .BR cgcc (1)
 .
diff --git a/token.h b/token.h
index ba7866d..44128f2 100644
--- a/token.h
+++ b/token.h
@@ -48,6 +48,7 @@ struct stream {

 extern int input_stream_nr;
 extern struct stream *input_streams;
+extern unsigned int tabstop;

 struct ident {
 	struct ident *next;	/* Hash chain of identifiers */
diff --git a/tokenize.c b/tokenize.c
index d154882..828a76f 100644
--- a/tokenize.c
+++ b/tokenize.c
@@ -25,6 +25,7 @@
 int input_stream_nr = 0;
 struct stream *input_streams;
 static int input_streams_allocated;
+unsigned int tabstop = 8;

 #define BUFSIZE (8192)

@@ -232,7 +233,7 @@ repeat:
 		goto repeat;
 	}

-	stream->pos++;
+	stream->pos += (c == '\t') ? (tabstop - stream->pos % tabstop) : 1;

 	if (c == '\n') {
 		stream->line++;
@@ -291,7 +292,7 @@ static inline int nextchar(stream_t *stream)
 	if (offset < stream->size) {
 		int c = stream->buffer[offset++];
 		static const char special[256] = {
-			['\r'] = 1, ['\n'] = 1, ['\\'] = 1
+			['\t'] = 1, ['\r'] = 1, ['\n'] = 1, ['\\'] = 1
 		};
 		if (!special[c]) {
 			stream->offset = offset;

Re: [PATCH v2] Add -ftabstop=WIDTH

[Junio C Hamano]

"Hannes Eder" <hannes@hanneseder.net> writes:

> diff --git a/lib.c b/lib.c
> index 059ba3b..221e7b8 100644
> --- a/lib.c
> +++ b/lib.c
> @@ -15,6 +15,8 @@
>  #include <string.h>
>  #include <unistd.h>
>  #include <assert.h>
> +#include <limits.h>
> +#include <errno.h>

These includes after Chris's simplification should not be needed, no?

Re: [PATCH v2] Add -ftabstop=WIDTH

[Christopher Li]

On Sat, Jan 3, 2009 at 4:01 AM, Junio C Hamano <junio@pobox.com> wrote:
> "Hannes Eder" <hannes@hanneseder.net> writes:
>
>> diff --git a/lib.c b/lib.c
>> index 059ba3b..221e7b8 100644
>> --- a/lib.c
>> +++ b/lib.c
>> @@ -15,6 +15,8 @@
>>  #include <string.h>
>>  #include <unistd.h>
>>  #include <assert.h>
>> +#include <limits.h>
>> +#include <errno.h>
>
> These includes after Chris's simplification should not be needed, no?

Good catch. I incorporate it as well.

While you are here. I have a question regarding how to publish patch series
with git.

Take this patch as example. I want to have some repository to allow
others to pull the early version of the patch for testing. I also want
to incorporate
changes into the patch itself.

If I am using patch series. That is easy, just republish a new series.

With git, I can:
1) always submit incremental changes to master branch. Then the patch will
    be fragmented. I want the clean up version of the history.
2) Use rebase or cherry-pick to update the master branch. That will change
    the history of the commit. it causes trouble for people who pull the earlier
   version of the patch.
3) Always use a new branch to publish a new patch series. Then people who
   pull it need to know which branch to pull. It seems that I want an alias for
   the branch. On the client side it can use the same alias to pull different
   branch. I haven't figure out how to do that with git yet.

Any suggestions?

Thanks

Chris

Re: [PATCH v2] Add -ftabstop=WIDTH

[Junio C Hamano]

"Christopher Li" <sparse@chrisli.org> writes:

> While you are here. I have a question regarding how to publish patch series
> with git.
>
> Take this patch as example. I want to have some repository to allow
> others to pull the early version of the patch for testing. I also want
> to incorporate
> changes into the patch itself.

A short answer is that git itself does not offer an official, integrated
way to manage the life cycle of a patch series natively.  People seem to
use additional Porcelains such as StGIT and guilt for these things, and
they are good way to privately manage the evolution of a patch series, but
I do not know how good they are about publishing and working with others.

I however personally haven't felt much need for anything outside of what
is natively supported by git.

> If I am using patch series. That is easy, just republish a new series.

How?

> With git, I can:
> 1) always submit incremental changes to master branch. Then the patch will
>     be fragmented. I want the clean up version of the history.
> 2) Use rebase or cherry-pick to update the master branch. That will change
>     the history of the commit. it causes trouble for people who pull the earlier
>    version of the patch.
> 3) Always use a new branch to publish a new patch series. Then people who
>    pull it need to know which branch to pull. It seems that I want an alias for
>    the branch. On the client side it can use the same alias to pull different
>    branch. I haven't figure out how to do that with git yet.
>
> Any suggestions?

I won't claim to have the magic bullet, but I'll try to describe how I
have been managing the patches submitted to git.git tree for the last
three years.

First thing to realize is that a typical non-trivial patch series has
three distinct phases in its lifecycle:

 (1) A patch series is floated as "how about this" but it is still very
     rough.  Its design may be suboptimal, and/or its implementation may
     be too buggy even to be tried by somebody who is not very dedicated.

     After such initial showing, people will suggest improvements, pick
     nits, and even a total re-design.

 (2) After iterations, the series will start to solidify.  Its design
     becomes something that people can agree on, and the initial trivial
     mistakes in the implementation are corrected, even if neither the
     design nor the implementation is perfect.  In short, the series
     turns into a testable shape, and it becomes viable to have other
     people on board to further polish it.

     People will continue improve, pick nits but a total re-design and
     re-implementation becomes less likely at this point.

 (3) After further iterations, the series becomes worthy to be in the
     official release.  It would be feature complete and without an
     obvious cosmetic issues.

     There may still be further enhancements, but at this point, they are
     just that: further enhancements and improvements, not fixes to a
     showstopper.

I personally consider that keeping track of the history of the patch
series in its infancy (i.e. phase #1) is not worth the trouble.  Early
mistakes that need a total redoing from scratch are not something worthy
of being kept in the history in the longer term.  On the other hand, once
the series becomes more solid (i.e. phase #2), it becomes more worthwhile
to keep track of the incremental evolution of the series.  A series that
passed these phases (i.e. in phase #3) of course are worth keeping track
of the incremental evolution, too.

For this reason, git.git has two development integration branches, 'next' and 'pu'.

A new series is always applied to its own topic branch, that is forked
from the oldest applicable release branch (not to be confused with the
development integration branches), which is one of:

 - 'maint', which is forked from the last major release --- as of this
   writing, it was forked from v1.6.1;

 - 'maint-X.Y.Z', which is forked from the stable release that is worthy
   of maintaining for the long term, and that is older than what 'maint'
   was forked from --- as of this writing, I have such branches for v1.5.4,
   v1.5.5, v1.5.6 and v1.6.0 release.

 - 'master', which builds on top of the last stable release to prepare for
   the next majore release --- as of this writing, it is preparing for
   v1.6.2;

When the series is still in phase #1, it is not worth keeping track of its
incremental development.  You _could_ do it, but when the series is so
slushy that it can get a total rewriting any time, it is not really worth
the trouble, as the history will not be incremental anyway at that point.

I merge such a topic branch to the integration branch called 'pu' (stands
for 'proposed updates'), and let the development community aware of the
fact that 'pu' itself, and any topic branch that is only in 'pu', are
subject to be rewound and rebuilt from scratch.  In other words, nobody is
supposed to build on it.  'pu' is for highly motivated and interested
parties to check new topics out and take a look at it.

When an update appears on the list for a topic that is only in 'pu', it is
submitter's choice to make it incremental (in which case I'll simply apply
them on top of the existing topic branch), or a complete replacement.
However, the submitters are encouraged to send in a replacement patch for
such a topic if the earlier mistakes their follow-up patch fixes are of
trivial and stupid kind that is not worth keeping in the history.  I may
even use an incremental patch to amend the existing commit (i.e. squash
the fix in to erase early mistakes).

A topic that was only in 'pu', with enough effort and luck, eventually may
become solid enough and move to phase #2.  When that happens, the topic is
merged to another integration branch called 'next' (it does *NOT* stand
for 'will be in the next major release').  The promise between I and the
development community is that topics that are merged to 'next' will get
only incremental updates and never rewound.  From then on, the topic is
refined incrementally until perfection.

When the topic moves to phase #3, it is merged to the 'master' branch.
It may further be merged down to older maintenance branches after it
proves useful/correct in the field.

Many trivially correct patches do not fully go through the above process
and are applied directly to the oldest, relevant release branch, such as
'maint' or 'master'.  To integrate these changes upward, periodically, the
tip of 'maint' is merged to 'master', and the tip of 'master' is merged to
'next'.

In other words, 'next' is where the real incremental development happens.
It merges updates to topic branches that have passed phase #1 (iow, worthy
of developing incrementally) and also updates to more stable release
branches.

Each time 'next' is updated, 'pu' is first reset to the tip of it.  The
topics that are still slushy and kept out of 'next' are updated by
replacing the commits by replacement patches.  Then these topiocs are
merged to 'pu', essentially rebuilding 'pu' branch from scratch.

My suggestion, if you think keeping track of early mistakes is worth
doing, is to use a variant of the way 'pu' is managed.  Instead of
declaring the topics in it subject to rewinding and rebuilding, you keep
track of the changes to them incrementally.  My gut feeling is that this
will probably make the development history unmanageably messy when a topic
leaves phase #1 and becomes worthy of bing in phase #2, and you may choose
to tidy up the history with "rebase -i", making sure that the end result
stays the same, immediately before merging the topic to 'next'.

Although, as I said, I do not personally think it is worth the trouble.

Re: [PATCH v2] Add -ftabstop=WIDTH

[Christopher Li]

Thank you very much for the detail explain of how you manage the git
release with
different branches. That is very helpful to me.

On Sun, Jan 4, 2009 at 1:05 AM, Junio C Hamano <junio@pobox.com> wrote:
>
>> If I am using patch series. That is easy, just republish a new series.
>
> How?

This is how Andrew Morton manage his -mm series. The heavy lifting
is done out side of git. As my understand,  Andrew using his own patch-series
script to manage changes in a series of patches. There is a series
file descriptor
what patch files are going to apply in to the tree. The patches can be
easily modify
(using push/pop/refresh) or drop out from the series completely. When Andrew
release a new "-mm" series. He just import the series into a git
branch and publish
that. The git repository is just a handy ftp replacement.

So my question can be simplify as, can I do what Andrew Morton do with his "-mm"
series using pure git?

> I won't claim to have the magic bullet, but I'll try to describe how I
> have been managing the patches submitted to git.git tree for the last
> three years.
>
> First thing to realize is that a typical non-trivial patch series has
> three distinct phases in its lifecycle:
>
>  (1) A patch series is floated as "how about this" but it is still very
>     rough.  Its design may be suboptimal, and/or its implementation may
>     be too buggy even to be tried by somebody who is not very dedicated.
>
>     After such initial showing, people will suggest improvements, pick
>     nits, and even a total re-design.
>
>  (2) After iterations, the series will start to solidify.  Its design
>     becomes something that people can agree on, and the initial trivial
>     mistakes in the implementation are corrected, even if neither the
>     design nor the implementation is perfect.  In short, the series
>     turns into a testable shape, and it becomes viable to have other
>     people on board to further polish it.
>
>     People will continue improve, pick nits but a total re-design and
>     re-implementation becomes less likely at this point.
>
>  (3) After further iterations, the series becomes worthy to be in the
>     official release.  It would be feature complete and without an
>     obvious cosmetic issues.
>
>     There may still be further enhancements, but at this point, they are
>     just that: further enhancements and improvements, not fixes to a
>     showstopper.

I totally agree with the 3 stage of the patch life cycle.

>
> I personally consider that keeping track of the history of the patch
> series in its infancy (i.e. phase #1) is not worth the trouble.  Early

I agree it is not worth while to keep the too early history. That is not
the problem. This problem is if I publish those early changes, others might
have pull from it. Then it come troublesome to modify the patch itself.

I can have clean patch history and it cause conflict when others sync to it.
Or I can have fragmented patch history(using incremental commit) but
others can sync to it smoothly. I can't have both.

> mistakes that need a total redoing from scratch are not something worthy
> of being kept in the history in the longer term.  On the other hand, once
> the series becomes more solid (i.e. phase #2), it becomes more worthwhile
> to keep track of the incremental evolution of the series.  A series that
> passed these phases (i.e. in phase #3) of course are worth keeping track
> of the incremental evolution, too.
>
> For this reason, git.git has two development integration branches, 'next' and 'pu'.

So here is my understanding of what you described. The 'pu' branch is for
highly experiment changes. The 'pu' branch can rewind and rewrite the
history. Once the patch merge to 'next', the history will not change any more.
All update will stay as incremental changes.

One question, does user suffer from conflict when then pull from the 'pu'
branch?

> My suggestion, if you think keeping track of early mistakes is worth
> doing, is to use a variant of the way 'pu' is managed.  Instead of

I don't care that much about keeping track of early mistakes. I just want to:
1) publish early.
2) able to fix the early mistake and maintain the history clean.

I think I will just adopt the 'pu' branch approach and declare that
'pu' branch might need rewind.

> declaring the topics in it subject to rewinding and rebuilding, you keep
> track of the changes to them incrementally.  My gut feeling is that this
> will probably make the development history unmanageably messy when a topic
> leaves phase #1 and becomes worthy of bing in phase #2, and you may choose
> to tidy up the history with "rebase -i", making sure that the end result
> stays the same, immediately before merging the topic to 'next'.
>
> Although, as I said, I do not personally think it is worth the trouble.

Thanks again for the suggestion. A 'pu' branch make sense.

I have some thought about cleaning the history without using a different branch.
When a patch come in, we often have pretty good idea how ready this patches
is. However, it is kind of arbitrate which stage it is in as well. Because once
it to to 'next' branch, there is no coming back. I might regret I
merge a patch to
'next' too early.

Here is an idea, I am just thinking it out loud.

Given 'pu' branch like this, (each [ ] is a commit, A1 is a follow up change for
A0).

'pu' branch: [A0] - [B0] - [A1] - [C0] -[B1] -[A2]

We can have a temporary clean up branch fork from 'pu' looks like this:

'tmp_clean' branch: [A0 + A1 + A2] - [B0 + B1] - [C0]

 'tmp_clean' and 'pu' will generate exactly the same tree. The
only different is the history path it take to get there.

Then we can have 'pu' merge from 'tmp_clean', with zero text
changes. The only change is the change log and we tell git
that the merge is for history clean up. So when we launch
"git log", by default it will follow the "tmp_clean" path rather
than the original "pu" path.

So it just provide "alternative" view of the history without introduce
real changes. When user pull from 'pu', it can automatically get the
cleanup version of the history without introduce conflicts.

It seems it can have the best of both worlds. I am not sure weather
it is doable or worth while to do though.

Chris

[PATCH v3] Add -ftabstop=WIDTH

[Hannes Eder]

Make tokenizer aware of tabstops and add the commandline option:

-ftabstop=WIDTH
    Set the distance between tab stops.  This helps sparse report correct
    column numbers in warnings or errors.  If the value is less than 1 or
    greater than 100, the option is ignored.  The default is 8.

With simplifications suggested by Christopher Li and Junio C Hamano.

Signed-off-by: Hannes Eder <hannes@hanneseder.net>
---

v3: fix a source of a future bugs in option parsing (introduced in v2),
i.e. if an option '-ffoo' is defined then '-ftabstop=foo' whould also
set the option foo.


 lib.c      |   19 +++++++++++++++++++
 sparse.1   |    7 +++++++
 token.h    |    1 +
 tokenize.c |    5 +++--
 4 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/lib.c b/lib.c
index 059ba3b..3185f9f 100644
--- a/lib.c
+++ b/lib.c
@@ -511,6 +511,25 @@ static char **handle_switch_f(char *arg, char **next)
 	int flag = 1;
 
 	arg++;
+
+	if (!strncmp(arg, "tabstop=", 8)) {
+		char *end;
+		unsigned long val;
+		arg += 8;
+
+		if (*arg == '\0')
+			die("error: missing argument to \"-ftabstop=\"");
+
+		/* we silently ignore silly values */
+		val = strtoul(arg, &end, 10);
+		if (*end == '\0' && 1 <= val && val <= 100)
+			tabstop = val;
+
+		return next;
+	}
+
+	/* handle switches w/ arguments above, boolean and only boolean below */
+
 	if (!strncmp(arg, "no-", 3)) {
 		flag = 0;
 		arg += 3;
diff --git a/sparse.1 b/sparse.1
index d242dc7..a9e3397 100644
--- a/sparse.1
+++ b/sparse.1
@@ -287,6 +287,13 @@ However, this behavior can lead to subtle errors.
 
 Sparse does not issue these warnings by default.
 .
+.SH OTHER OPTIONS
+.TP
+.B \-ftabstop=WIDTH
+Set the distance between tab stops.  This helps sparse report correct
+column numbers in warnings or errors.  If the value is less than 1 or
+greater than 100, the option is ignored.  The default is 8.
+.
 .SH SEE ALSO
 .BR cgcc (1)
 .
diff --git a/token.h b/token.h
index ba7866d..44128f2 100644
--- a/token.h
+++ b/token.h
@@ -48,6 +48,7 @@ struct stream {
 
 extern int input_stream_nr;
 extern struct stream *input_streams;
+extern unsigned int tabstop;
 
 struct ident {
 	struct ident *next;	/* Hash chain of identifiers */
diff --git a/tokenize.c b/tokenize.c
index d154882..828a76f 100644
--- a/tokenize.c
+++ b/tokenize.c
@@ -25,6 +25,7 @@
 int input_stream_nr = 0;
 struct stream *input_streams;
 static int input_streams_allocated;
+unsigned int tabstop = 8;
 
 #define BUFSIZE (8192)
 
@@ -232,7 +233,7 @@ repeat:
 		goto repeat;
 	}
 
-	stream->pos++;
+	stream->pos += (c == '\t') ? (tabstop - stream->pos % tabstop) : 1;
 
 	if (c == '\n') {
 		stream->line++;
@@ -291,7 +292,7 @@ static inline int nextchar(stream_t *stream)
 	if (offset < stream->size) {
 		int c = stream->buffer[offset++];
 		static const char special[256] = {
-			['\r'] = 1, ['\n'] = 1, ['\\'] = 1
+			['\t'] = 1, ['\r'] = 1, ['\n'] = 1, ['\\'] = 1
 		};
 		if (!special[c]) {
 			stream->offset = offset;

Re: [PATCH v3] Add -ftabstop=WIDTH

[Christopher Li]

On Thu, Jan 8, 2009 at 11:18 AM, Hannes Eder <hannes@hanneseder.net> wrote:

> +
> +       if (!strncmp(arg, "tabstop=", 8)) {
> +               char *end;
> +               unsigned long val;
> +               arg += 8;
> +
> +               if (*arg == '\0')
> +                       die("error: missing argument to \"-ftabstop=\"");
> +
> +               /* we silently ignore silly values */
> +               val = strtoul(arg, &end, 10);
> +               if (*end == '\0' && 1 <= val && val <= 100)
> +                       tabstop = val;
> +
> +               return next;
> +       }
> +
> +       /* handle switches w/ arguments above, boolean and only boolean below */
> +
>        if (!strncmp(arg, "no-", 3)) {
>                flag = 0;
>                arg += 3;

Is that the only portion that get changed between v2 and v3?
It took me a while to realized what really get changed here.

I suggest a new function: handle_switch_ftabstop() here.

Then we do:

        if (!strncmp(arg, "tabstop=", 8))
            return handle_switch_ftabstop(arg+8, next);

It will make handle_switch_f cleaner.

I already apply your V2 patch. Can you make this change
an incremental patch against my tree?

Thanks

Chris

[PATCH] refactor handle_switch_f

[Hannes Eder]

This also fixes a possible source of bugs in parsing other
-f<whatever> options, i.e. -ftabstop=foo would set the option -ffoo.

Signed-off-by: Hannes Eder <hannes@hanneseder.net>
---

On Thu, Jan 8, 2009 at 9:13 PM, Christopher Li <sparse@chrisli.org> wrote:
> Is that the only portion that get changed between v2 and v3?
> It took me a while to realized what really get changed here.

Yes, only handle_switch_f has changed between v2 and v3.

> I suggest a new function: handle_switch_ftabstop() here.
>
> Then we do:
>
>       if (!strncmp(arg, "tabstop=", 8))
>           return handle_switch_ftabstop(arg+8, next);
>
> It will make handle_switch_f cleaner.

Good idea.

> I already apply your V2 patch. Can you make this change
> an incremental patch against my tree?

Here we go.

 lib.c |   31 ++++++++++++++++++++-----------
 1 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/lib.c b/lib.c
index be9e8d7..54d8cf5 100644
--- a/lib.c
+++ b/lib.c
@@ -524,25 +524,34 @@ static char **handle_switch_O(char *arg, char **next)
 	return next;
 }
 
+static char **handle_switch_ftabstop(char *arg, char **next)
+{
+	char *end;
+	unsigned long val;
+
+	if (*arg == '\0')
+		die("error: missing argument to \"-ftabstop=\"");
+
+	/* we silently ignore silly values */
+	val = strtoul(arg, &end, 10);
+	if (*end == '\0' && 1 <= val && val <= 100)
+		tabstop = val;
+
+	return next;
+}
+
 static char **handle_switch_f(char *arg, char **next)
 {
 	int flag = 1;
 
 	arg++;
 
-	if (!strncmp(arg, "tabstop=", 8)) {
-		char *end;
-		unsigned long val;
-		arg += 8;
+	if (!strncmp(arg, "tabstop=", 8))
+		return handle_switch_ftabstop(arg+8, next);
 
-		if (*arg == '\0')
-			die("error: missing argument to \"-ftabstop=\"");
+	/* handle switches w/ arguments above, boolean and only boolean below */
 
-		/* we silently ignore silly values */
-		val = strtoul(arg, &end, 10);
-		if (*end == '\0' && 1 <= val && val <= 100)
-			tabstop = val;
-	} else if (!strncmp(arg, "no-", 3)) {
+	if (!strncmp(arg, "no-", 3)) {
 		flag = 0;
 		arg += 3;
 	}

Re: [PATCH] refactor handle_switch_f

[Christopher Li]

On Thu, Jan 8, 2009 at 12:50 PM, Hannes Eder <hannes@hanneseder.net> wrote:
> This also fixes a possible source of bugs in parsing other
> -f<whatever> options, i.e. -ftabstop=foo would set the option -ffoo.
>
> Signed-off-by: Hannes Eder <hannes@hanneseder.net>

Applied.

Chris

Re: [PATCH v2] Add -ftabstop=WIDTH

[Christopher Li]

On Fri, Jan 2, 2009 at 6:22 AM, Hannes Eder <hannes@hanneseder.net> wrote:
> I like the part in tokenizer.c, I even simplified it a bit further, to
> use a ternary op, and
> do not use a variable.

I applied your patch with change suggested by Junio C Hamano.

Chris