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

Compose parser is too lenient #631

Open
wismill opened this issue Jan 30, 2025 · 2 comments
Open

Compose parser is too lenient #631

wismill opened this issue Jan 30, 2025 · 2 comments
Labels
compose Indicates a need for improvements or additions to Compose handling enhancement Indicates new feature requests

Comments

@wismill
Copy link
Member

wismill commented Jan 30, 2025

The following file parses correctly, although no sequence is valid!

! include ~
! include x
! include ""
include
include !
include x
# include ""
:
a :
: a
<a> : a a
!:
<a> : !

We should fail at least on the includes.

Found while looking for a Compose compile failure in #360.

@wismill wismill added compose Indicates a need for improvements or additions to Compose handling enhancement Indicates new feature requests labels Jan 30, 2025
@bluetech
Copy link
Member

There is a max of 10 errors:

enum { MAX_ERRORS = 10 };

If I understand, you're thinking

  1. If no valid sequences at all at the end, error.
  2. If include fails, fail fast.

(1) sounds sensible to me. (2) might be a backward compat issue though. I can imagine people having some bad includes which they ignore but now the file will stop loading. But maybe it's OK to break?

@wismill
Copy link
Member Author

wismill commented Jan 30, 2025

The are two issues with includes here:

  • bad syntax: it should fail early. We may be lenient for some other syntax issues, but I think we should not for includes.
  • file not found: skip and emit an error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compose Indicates a need for improvements or additions to Compose handling enhancement Indicates new feature requests
Projects
None yet
Development

No branches or pull requests

2 participants