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

WELD-2408 Beans.xml with only comment will be treated as an empty bea… #1708

Closed
wants to merge 1 commit into from

Conversation

manovotn
Copy link
Contributor

…ns.xml

Technically speaking, XML containing only comments is invalid (according to w3c scheme which we use) but so is empty XML file and CDI spec requires empty file to work.
The question is then, what does empty mean? No chars at all, or no meaningful elements?

I just played around with what we could do to make this work, not sure we should do it at all. Opinions are welcome.

@rdebusscher
Copy link

I agree that a file containing only a comment is strictly speaking not valid.

But since an empty file is supported, it seems logical to me that comments are ignored to determine if the file is empty or not (just like comments are ignored by the Java compiler)

@manovotn manovotn requested review from mkouba and tremes July 19, 2017 12:09
Copy link
Contributor

@tremes tremes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure whether this can have some significant negative performance implications (hopefully not), but otherwise it looks OK.

@manovotn
Copy link
Contributor Author

The loop breaks as soon as we find any non-comment tag. The only real overhead is opening the input stream twice. Shouldn't be that much unless you plan to write a novel in a form of a beans.xml comment.

@mkouba
Copy link
Member

mkouba commented Jul 24, 2017

I tend to disagree with this solution. An empty file is just a marker. Once you add a content it must be parsed and so I think it's not a problem when it fails for an invalid xml file. Using <!-- empty beans xml --> <beans/> is the way to go.

@mkouba
Copy link
Member

mkouba commented Jul 24, 2017

WELD-2408 was rejected.

@mkouba mkouba closed this Jul 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants