New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ocamlbuild, improve _tags parsing and error report #6087
Comments
Comment author: @gasche 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). |
Comment author: meyer 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. |
Comment author: @dbuenzli (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. |
Comment author: meyer 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. |
Comment author: meyer 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) |
Comment author: @dbuenzli Note error also occurs when you have a space (0x20) at the end of a _tags file line. Could you also check that please.
|
Comment author: @damiendoligez 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 #4598) |
Comment author: @dbuenzli Since it has to change anyways why not switch GNU style error messages (which are detected by default by emacs's compilation mode) ? |
Comment author: @damiendoligez That's what I intend to do. |
Comment author: @gasche 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. |
Original bug ID: 6087
Reporter: @dbuenzli
Assigned to: @gasche
Status: closed (set by @xavierleroy on 2016-12-07T10:34:32Z)
Resolution: fixed
Priority: normal
Severity: minor
Version: 4.00.0
Target version: 4.02.0+dev
Category: -for ocamlbuild use https://github.com/ocaml/ocamlbuild/issues
Tags: junior_job
Related to: #4598 #5212 #6518
Monitored by: meyer @gasche @dbuenzli
Bug description
Great to see there are people willing to improve ocamlbuild !
So here are a few things:
true : package(gg),
package(vg)
true :
package(gg), package(vg)
true
: package(gg), package(vg)
Could we
a)
Either be more consistant in the \ treatement or even better, avoid it altogether (I don't know if it would render the grammar ambiguous)
b)
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")
The text was updated successfully, but these errors were encountered: