[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 2/4] errors: show carets
From: |
Akim Demaille |
Subject: |
Re: [PATCH 2/4] errors: show carets |
Date: |
Tue, 4 Dec 2012 15:29:17 +0100 |
Le 4 déc. 2012 à 15:26, Theophile Ranquet <address@hidden> a écrit :
> diff --git a/src/location.c b/src/location.c
> index fa1b53c..3c3bc6e 100644
> --- a/src/location.c
> +++ b/src/location.c
> @@ -138,6 +138,77 @@ location_print (FILE *out, location loc)
> return res;
> }
>
> +struct caret_info
> +{
> + FILE* source;
> + size_t line;
> + size_t offset;
> +};
Please, comment all this (structure, and what follows).
> +
> +static struct caret_info caret_info = { NULL, 1, 0 };
> +
> +void
> +cleanup_caret ()
> +{
> + if (caret_info.source)
> + fclose (caret_info.source);
> +}
> +
> +void
> +location_caret (FILE *out, location loc)
> +{
> + int i;
Please, use shorter scopes for this 'i'.
> + char buf[1024];
> +
> + /* FIXME: find a way to support X-file locations, and only open once each
> + file. That would make the procedure future-proof. */
> + if (! (caret_info.source
> + || (caret_info.source = fopen (loc.start.file, "r")))
Don't you have to fclose before fopen?
> + || loc.start.column > sizeof buf)
> + return;
> +
> + /* If the line we want to quote is seekable (the same line as the previous
> + location), just seek it. If it was before, we lost track of it, so
> + return to the start of file. */
> + if (caret_info.line <= loc.start.line)
> + fseek (caret_info.source, caret_info.offset, SEEK_SET);
> + else
> + {
> + caret_info.line = 1, caret_info.offset = 0;
Two stmts please.
> + fseek (caret_info.source, caret_info.offset, SEEK_SET);
> + }
> +
> + /* Advance to the line's position, keeping track of the offset. */
> + for (i = caret_info.line; i < loc.start.line; caret_info.offset++)
> + if (fgetc (caret_info.source) == '\n')
> + ++i;
> + caret_info.line = loc.start.line;
> +
> + /* Read the actual line. Don't update the offset, so that we keep a
> pointer
> + to the start of the line. */
> + if (fgets (buf, sizeof buf, caret_info.source))
> + {
> + size_t len = strlen (buf);
fgets will not support \0. Can't we use getline instead?
It should also address some limitations of this implementation
(1024 for instance, although I agree that at some point in
width, caret diagnostics are no longer useful).
> + /* The caret of a multiline location ends with the first line. */
> + int end = loc.start.line != loc.end.line ? len : loc.end.column;
> +
> + if (len)
> + {
> + /* Quote the file, by displaying the read line prefixed and
> + suffixed with the indentation, here 1 character. */
> + buf[len - 1] = '\n';
> + fprintf (out, " %s ", buf);
> +
> + /* Print the caret. */
> + for (i = 1; i < loc.start.column; ++i)
> + fprintf (out, " ");
fprintf (out, "%*s", loc.start.column - 1, "");
> + for (; i < end || i == loc.start.column; ++i)
> + fprintf (out, "^");
putc should suffice.
> + }
> + fprintf (out, "\n");
Ditto.
> + }
> +}
> +
> void
> boundary_set_from_string (boundary *bound, char *loc_str)
> {
> diff --git a/src/location.h b/src/location.h
> index 5ebb92e..3bd44be 100644
> --- a/src/location.h
> +++ b/src/location.h
> @@ -102,6 +102,11 @@ void location_compute (location *loc,
> characters. */
> unsigned location_print (FILE *out, location loc);
>
> +void cleanup_caret (void);
Please, doc.
> +
> +/* Output to OUT the line and caret corresponding to location LOC. */
> +void location_caret (FILE *out, location loc);
> +
> /* Return -1, 0, 1, depending whether a is before, equal, or
> after b. */
> static inline int
> diff --git a/src/main.c b/src/main.c
> index 0396b0f..184d789 100644
> --- a/src/main.c
> +++ b/src/main.c
> @@ -216,5 +216,7 @@ main (int argc, char *argv[])
> timevar_stop (TV_TOTAL);
> timevar_print (stderr);
>
> + cleanup_caret ();
> +
> return complaint_issued ? EXIT_FAILURE : EXIT_SUCCESS;
> }
> --
> 1.8.0
>
[PATCH 4/4] doc: document carets, Theophile Ranquet, 2012/12/04
[PATCH 3/4] tests: check carets, Theophile Ranquet, 2012/12/04
Re: [PATCH 0/4] {maint} caret errors, Akim Demaille, 2012/12/04