Skip to content
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

strict comment syntax #16

Open
vgheo opened this issue Sep 13, 2019 · 4 comments
Open

strict comment syntax #16

vgheo opened this issue Sep 13, 2019 · 4 comments

Comments

@vgheo
Copy link
Contributor

vgheo commented Sep 13, 2019

The current implementation of the parser for excel comments is a handcrafted parser that tolerates (ignores) free-text comments mixed with concordion syntax.

As a consequence, errors in the excel/concordion comments propagate to the intermediate html and are reported as errors at that level, with no traceability to the excel source location.
This makes debugging of syntax issues very tedious.
Examples:

  • accidentally introduced a similar-but-different character for single-quote
  • have two successive single-quotes that look identical to double-quote
  • (i bet users can come with loads of other examples... )

As an enhancement, the following is proposed

  • introduce a grammar-based parser for excel/concordion comments ( eg using https://www.antlr.org/ )
  • extend the @extension annotation such that to allow the user to specify whether to use the "strict" comment parser (with better diagnosis) or the "lax" parser (current one - tolerant to freetext)
@vgheo
Copy link
Contributor Author

vgheo commented Sep 13, 2019

@robmoffat
Copy link
Member

I think you’re right that error handling is less than perfect.

The problem is that the conversion leaves none of the context of where the errors exist in the spreadsheet. Is that correct? I’m a bit hazy on this now as I haven’t used it in a while

I’m not entirely sure how using a grammar would fix this?

@vgheo
Copy link
Contributor Author

vgheo commented Sep 14, 2019

I see several problems.

Considering all errors that can occur in a concordion/excel specification, these can be categorized as follows:
A. excel comment syntax errors

  • eg. mismatched quotes
  • (current issue) - these can be detected by a parser

B. excel to html translation errors

C. html syntax errors

  • I think these should never occur, provided parser at point A is strict enough

D. Concordion execution errors

I could consider contributing a solution, if you agree in principle that the enhacement makes sense and maybe provide some guidance.

@robmoffat
Copy link
Member

This is excellent analysis, thanks.

Regarding (A), you could go down that route, but it might actually be less effort to just ruggedise the existing code. The coverage isn't wonderful at the moment:

Screenshot 2019-09-15 at 20 17 17

(B) we discussed in #15 the exception handing (and debuggability of the html generally) could be improved by included information about where the HTML came from. Would that help with (D) too?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants