|Anonymous | Login | Signup for a new account||2014-12-19 22:00 CET|
|Main | My View | View Issues | Change Log | Roadmap|
|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0006087||OCaml||OCamlbuild (the tool)||public||2013-07-26 14:51||2014-08-21 07:47|
|Priority||normal||Severity||minor||Reproducibility||have not tried|
|Target Version||4.02.0+dev||Fixed in Version|
|Summary||0006087: ocamlbuild, improve _tags parsing and error report|
|Description||Great to see there are people willing to improve ocamlbuild ! |
So here are a few things:
1) The following _tags file works:
true : package(gg), \
2) But not the following one:
true : \
3) Neither the following one:
: package(gg), package(vg)
Either be more consistant in the \ treatement or even better, avoid it altogether (I don't know if it would render the grammar ambiguous)
Have a gnu-style error messages (http://www.gnu.org/prep/standards/standards.html#Errors [^])
so that we can easily jump to the location of the error from our editors. This is what is currently reported:
Lexical analysis error: _tags: Bad key in configuration line at line 1 (from file: "_tags")
Thanks, this kind of bug report is indeed helpful to give us a concrete repro case to work on.
(a) should definitely be fixed (\ should not change the validity of the syntax unless it splits words)
I'm not sure about (b) GNU style error messages; I think we should rather follow the global convention of the OCaml compiler error messages (which means that if you run ocamlbuild from any place that knows how to parse those messages, such as Emacs "compile" mode, you'll get the location parsed as well). Enabling GNU style error messages instead of the current one OCaml uses is a distribution-wide issue that has been discussed in the past (I think having patches for this would be interesting, but it's not on my radar of urgent things to do right now).
Regarding (b): Maybe we can provide a flag for GNU message format. It's additional maintainance cost but probably relaively minimal.
I'd do the same for the rest of the toolchain where it's possible, but not that that I don't like the current situation.
|(b) Having what the general toolchain does is fine with me and I'd rather avoid adding new flags to the tools for that kind of details.|
|OK, we should then be fixing it, so it behaves like the rest of the system, along with the newline separator fix. Thanks for the report.|
Let's tentatively schedule the fix for the release, it might slip away, but at least better support for Emacs mode is important, and comes at minimal cost and risk.
The other interesing way of dealing it with newlines would be via identation, so the spaces in the begining of line would be significant. (my mistake I am reporting it at the moment)
Note error also occurs when you have a space (0x20) at the end of a _tags file line. Could you also check that please.
> hexdump -C _tags
00000000 74 72 75 65 3a 20 70 61 63 6b 61 67 65 28 67 67 |true: package(gg|
00000010 29 20 |) |
> ocamlbuild test.native
Lexical analysis error: _tags: Bad values in configuration line at line 1 (from file: "_tags")
Note for the future: the toolchain's error report format will have to be changed at some point because it gives: (filename, line, char1, char2) where char1 and char2 relative to the beginning of line, while emacs wants (filename, line1, char1, line2, char2) where char2 is relative to the beginning of line2. (see 0004598)
|Since it has to change anyways why not switch GNU style error messages (which are detected by default by emacs's compilation mode) ?|
|That's what I intend to do.|
edited on: 2014-08-21 07:41
Regarding the _tags syntax: I went with (a) make escape-newline placement more consistent, because I don't want to try to guess whether a given blurb is a pattern or a tag (it depends on whether a ':' follows, which does not cope very well with the fact that those are currently implemented with no-readahead lexers).
Patch 4.02@15118 makes all three examples accepted by ocamlbuild. These are in fact the only cases that have changed (escaped newlines before and after the ':'), so there were no consistency improvements in the rest of the grammar. But I didn't see any other place where accepting escaped newline would be natural.
The computation of error positions in presence of escaped newlines was also improved.
I think we can consider this particular PR resolved. If it doesn't already exists, I will open a new PR regarding the error-message format (so that we don't lose track of this issue), and you should feel free to open new PRs for other inconsistencies in the ocamlbuild parsing behavior.
|2013-07-26 14:51||dbuenzli||New Issue|
|2013-07-26 18:35||gasche||Relationship added||related to 0005212|
|2013-07-26 18:40||gasche||Note Added: 0009869|
|2013-07-26 18:40||gasche||Assigned To||=> gasche|
|2013-07-26 18:40||gasche||Status||new => confirmed|
|2013-07-26 18:40||gasche||Assigned To||gasche =>|
|2013-07-26 18:41||gasche||Assigned To||=> gasche|
|2013-07-26 18:41||gasche||Status||confirmed => assigned|
|2013-07-26 18:41||gasche||Target Version||=> 4.01.1+dev|
|2013-07-26 19:04||meyer||Note Added: 0009871|
|2013-07-26 19:12||dbuenzli||Note Added: 0009872|
|2013-07-26 19:38||meyer||Note Added: 0009873|
|2013-07-26 19:38||meyer||Target Version||4.01.1+dev => 4.01.0+dev|
|2013-07-26 19:41||meyer||Note Added: 0009874|
|2013-07-28 09:05||gasche||Tag Attached: junior_job|
|2013-07-29 16:20||dbuenzli||Note Added: 0009984|
|2013-08-19 16:42||doligez||Note Added: 0010199|
|2013-08-19 16:42||doligez||Target Version||4.01.0+dev => 4.01.1+dev|
|2013-08-19 16:42||doligez||Relationship added||related to 0004598|
|2013-08-19 17:00||dbuenzli||Note Added: 0010202|
|2014-05-25 20:20||doligez||Target Version||4.01.1+dev => 4.02.0+dev|
|2014-07-24 22:55||doligez||Note Added: 0011908|
|2014-08-21 07:41||gasche||Note Added: 0012033|
|2014-08-21 07:41||gasche||Status||assigned => resolved|
|2014-08-21 07:41||gasche||Resolution||open => fixed|
|2014-08-21 07:41||gasche||Note Edited: 0012033||View Revisions|
|2014-08-21 07:47||gasche||Relationship added||related to 0006518|
|Copyright © 2000 - 2011 MantisBT Group|