You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@maven.apache.org by he...@free.fr on 2015/08/11 10:04:03 UTC
Re: svn commit: r1695142 -
/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
wow, I don't like this
in XML, encoding is self provided
with such feature, an XML-invalid document can be read by Maven (and Maven only, since it is XML-invalid)
I'm -1 on this: we can't help people make Maven-specific pseudo XML
Regards,
Hervé
----- Mail original -----
De: ptahchiev@apache.org
À: commits@maven.apache.org
Envoyé: Lundi 10 Août 2015 20:00:00
Objet: svn commit: r1695142 - /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
Author: ptahchiev
Date: Mon Aug 10 18:00:00 2015
New Revision: 1695142
URL: http://svn.apache.org/r1695142
Log:
Check for user's provided encoding, and only if it's null then use the encoding of the xml document. Closes [DOXIASITETOOLS-104]
Modified:
maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
Modified: maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
URL: http://svn.apache.org/viewvc/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java?rev=1695142&r1=1695141&r2=1695142&view=diff
==============================================================================
--- maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java (original)
+++ maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java Mon Aug 10 18:00:00 2015
@@ -389,7 +389,14 @@ public class DefaultSiteRenderer
switch ( parser.getType() )
{
case Parser.XML_TYPE:
- reader = ReaderFactory.newXmlReader( doc );
+ if ( siteContext.getInputEncoding() != null )
+ {
+ reader = ReaderFactory.newReader( doc, siteContext.getInputEncoding() );
+ }
+ else
+ {
+ reader = ReaderFactory.newXmlReader( doc );
+ }
if ( siteContext.isValidate() )
{
reader = validate( reader, resource );
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: svn commit: r1695142 -
/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
Posted by he...@free.fr.
Hi Lukas: nice to se that it has catched your interest :)
Yes, DOXIA-436 is the ideal, where we completely remove any Xhtml intermediate
The idea here is less ambitious: just avoid extending XhtmlParser, since the source is not Xhtml/Xml
But of course, don't hesitate to help!
Regards,
Hervé
----- Mail original -----
De: "Lukas Theussl" <lt...@gmail.com>
À: "Maven Developers List" <de...@maven.apache.org>
Envoyé: Jeudi 13 Août 2015 10:30:21
Objet: Re: svn commit: r1695142 - /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
see related https://issues.apache.org/jira/browse/DOXIA-436
Greetings! :)
-Lukas
Am 12.08.2015 um 23:54 schrieb herve.boutemy@free.fr:
> now that you told it, I'd seriously prefer change Doxia Markdown parser to use an XhtmlParser instance internally instead of extending XhtmlParser while completely replacing content parsed by the Xhtml parser: this would be a lot more clear (and would avoid adding bloat to getType())
>
> I didn't really try, I don't know if this change is really complex
>
> did you try?
>
> Regards,
>
> Hervé
>
> ----- Mail original -----
> De: "Petar Tahchiev" <pa...@gmail.com>
> À: "Maven Developers List" <de...@maven.apache.org>
> Envoyé: Mercredi 12 Août 2015 08:39:12
> Objet: Re: svn commit: r1695142 - /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>
> OK,
>
> sorry I wasn't aware that user specifying an input encoding for xml file
> would be considered as introducing a bug. Great for the test-case - I will
> revert my changes and work for a fix in the MarkdownParser. Would
> overriding the getType() method of the MarkdownParser be considered as a
> valid solution?
>
> 2015-08-12 2:42 GMT+03:00 <he...@free.fr>:
>
>> IIUC, your concerns are about Mardown: if Markdown parser has a bug, don't
>> hesitate to fix it
>> but do not break content for normal XML parsers, like fml or xdoc
>>
>> since your change did not make unit tests fail, this proves unit tests are
>> too weak: I just improved them in r1695408 to fail (and show clearly what
>> you are breaking)
>> and I reopened DOXIASITETOOLS-104
>>
>> You're probably right that making Markdown parser *extend* XhtmlParser is
>> probably wrong: it should *use* an XhtmlParser, but not extend it
>>
>> Regards,
>>
>> Hervé
>>
>> ----- Mail original -----
>> De: "Petar Tahchiev" <pa...@gmail.com>
>> À: "Maven Developers List" <de...@maven.apache.org>
>> Envoyé: Mardi 11 Août 2015 11:36:28
>> Objet: Re: svn commit: r1695142 -
>> /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>
>> Hi Herve,
>>
>> I did this because seems like a parser can be of type XML even if it's not
>> parsing only XML - for example the MarkdownParser (which is in doxia and
>> extends from the XmlParser) getType() returns 2 (XML parser type). I guess
>> there are two ways to go here - 1) first would be to allow the user to
>> force an encoding. It's his/hers decision and he/she takes the
>> responsibility. 2) Would be to override the XmlParser:getType() method in
>> MarkdownParser and make it return 0 (UNKNOWN_TYPE). To me this would lead
>> to inconsistency, because the MarkdownParser extends from XmlParser, but
>> returns another type. Furthermore I don't agree markdown syntax is in fact
>> xml syntax.
>>
>>
>> 2015-08-11 11:04 GMT+03:00 <he...@free.fr>:
>>
>>> wow, I don't like this
>>> in XML, encoding is self provided
>>>
>>> with such feature, an XML-invalid document can be read by Maven (and
>> Maven
>>> only, since it is XML-invalid)
>>>
>>> I'm -1 on this: we can't help people make Maven-specific pseudo XML
>>>
>>> Regards,
>>>
>>> Hervé
>>>
>>> ----- Mail original -----
>>> De: ptahchiev@apache.org
>>> À: commits@maven.apache.org
>>> Envoyé: Lundi 10 Août 2015 20:00:00
>>> Objet: svn commit: r1695142 -
>>>
>> /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>>
>>> Author: ptahchiev
>>> Date: Mon Aug 10 18:00:00 2015
>>> New Revision: 1695142
>>>
>>> URL: http://svn.apache.org/r1695142
>>> Log:
>>> Check for user's provided encoding, and only if it's null then use the
>>> encoding of the xml document. Closes [DOXIASITETOOLS-104]
>>>
>>> Modified:
>>>
>>>
>> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>>
>>> Modified:
>>>
>> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>> URL:
>>>
>> http://svn.apache.org/viewvc/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java?rev=1695142&r1=1695141&r2=1695142&view=diff
>>>
>>>
>> ==============================================================================
>>> ---
>>>
>> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>> (original)
>>> +++
>>>
>> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>> Mon Aug 10 18:00:00 2015
>>> @@ -389,7 +389,14 @@ public class DefaultSiteRenderer
>>> switch ( parser.getType() )
>>> {
>>> case Parser.XML_TYPE:
>>> - reader = ReaderFactory.newXmlReader( doc );
>>> + if ( siteContext.getInputEncoding() != null )
>>> + {
>>> + reader = ReaderFactory.newReader( doc,
>>> siteContext.getInputEncoding() );
>>> + }
>>> + else
>>> + {
>>> + reader = ReaderFactory.newXmlReader( doc );
>>> + }
>>> if ( siteContext.isValidate() )
>>> {
>>> reader = validate( reader, resource );
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>>
>>
>> --
>> Regards, Petar!
>> Karlovo, Bulgaria.
>> ---
>> Public PGP Key at:
>> http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x19658550C3110611
>> Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: svn commit: r1695142 - /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
Posted by Lukas Theussl <lt...@gmail.com>.
see related https://issues.apache.org/jira/browse/DOXIA-436
Greetings! :)
-Lukas
Am 12.08.2015 um 23:54 schrieb herve.boutemy@free.fr:
> now that you told it, I'd seriously prefer change Doxia Markdown parser to use an XhtmlParser instance internally instead of extending XhtmlParser while completely replacing content parsed by the Xhtml parser: this would be a lot more clear (and would avoid adding bloat to getType())
>
> I didn't really try, I don't know if this change is really complex
>
> did you try?
>
> Regards,
>
> Hervé
>
> ----- Mail original -----
> De: "Petar Tahchiev" <pa...@gmail.com>
> À: "Maven Developers List" <de...@maven.apache.org>
> Envoyé: Mercredi 12 Août 2015 08:39:12
> Objet: Re: svn commit: r1695142 - /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>
> OK,
>
> sorry I wasn't aware that user specifying an input encoding for xml file
> would be considered as introducing a bug. Great for the test-case - I will
> revert my changes and work for a fix in the MarkdownParser. Would
> overriding the getType() method of the MarkdownParser be considered as a
> valid solution?
>
> 2015-08-12 2:42 GMT+03:00 <he...@free.fr>:
>
>> IIUC, your concerns are about Mardown: if Markdown parser has a bug, don't
>> hesitate to fix it
>> but do not break content for normal XML parsers, like fml or xdoc
>>
>> since your change did not make unit tests fail, this proves unit tests are
>> too weak: I just improved them in r1695408 to fail (and show clearly what
>> you are breaking)
>> and I reopened DOXIASITETOOLS-104
>>
>> You're probably right that making Markdown parser *extend* XhtmlParser is
>> probably wrong: it should *use* an XhtmlParser, but not extend it
>>
>> Regards,
>>
>> Hervé
>>
>> ----- Mail original -----
>> De: "Petar Tahchiev" <pa...@gmail.com>
>> À: "Maven Developers List" <de...@maven.apache.org>
>> Envoyé: Mardi 11 Août 2015 11:36:28
>> Objet: Re: svn commit: r1695142 -
>> /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>
>> Hi Herve,
>>
>> I did this because seems like a parser can be of type XML even if it's not
>> parsing only XML - for example the MarkdownParser (which is in doxia and
>> extends from the XmlParser) getType() returns 2 (XML parser type). I guess
>> there are two ways to go here - 1) first would be to allow the user to
>> force an encoding. It's his/hers decision and he/she takes the
>> responsibility. 2) Would be to override the XmlParser:getType() method in
>> MarkdownParser and make it return 0 (UNKNOWN_TYPE). To me this would lead
>> to inconsistency, because the MarkdownParser extends from XmlParser, but
>> returns another type. Furthermore I don't agree markdown syntax is in fact
>> xml syntax.
>>
>>
>> 2015-08-11 11:04 GMT+03:00 <he...@free.fr>:
>>
>>> wow, I don't like this
>>> in XML, encoding is self provided
>>>
>>> with such feature, an XML-invalid document can be read by Maven (and
>> Maven
>>> only, since it is XML-invalid)
>>>
>>> I'm -1 on this: we can't help people make Maven-specific pseudo XML
>>>
>>> Regards,
>>>
>>> Hervé
>>>
>>> ----- Mail original -----
>>> De: ptahchiev@apache.org
>>> À: commits@maven.apache.org
>>> Envoyé: Lundi 10 Août 2015 20:00:00
>>> Objet: svn commit: r1695142 -
>>>
>> /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>>
>>> Author: ptahchiev
>>> Date: Mon Aug 10 18:00:00 2015
>>> New Revision: 1695142
>>>
>>> URL: http://svn.apache.org/r1695142
>>> Log:
>>> Check for user's provided encoding, and only if it's null then use the
>>> encoding of the xml document. Closes [DOXIASITETOOLS-104]
>>>
>>> Modified:
>>>
>>>
>> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>>
>>> Modified:
>>>
>> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>> URL:
>>>
>> http://svn.apache.org/viewvc/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java?rev=1695142&r1=1695141&r2=1695142&view=diff
>>>
>>>
>> ==============================================================================
>>> ---
>>>
>> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>> (original)
>>> +++
>>>
>> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>>> Mon Aug 10 18:00:00 2015
>>> @@ -389,7 +389,14 @@ public class DefaultSiteRenderer
>>> switch ( parser.getType() )
>>> {
>>> case Parser.XML_TYPE:
>>> - reader = ReaderFactory.newXmlReader( doc );
>>> + if ( siteContext.getInputEncoding() != null )
>>> + {
>>> + reader = ReaderFactory.newReader( doc,
>>> siteContext.getInputEncoding() );
>>> + }
>>> + else
>>> + {
>>> + reader = ReaderFactory.newXmlReader( doc );
>>> + }
>>> if ( siteContext.isValidate() )
>>> {
>>> reader = validate( reader, resource );
>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>>> For additional commands, e-mail: dev-help@maven.apache.org
>>>
>>>
>>
>>
>> --
>> Regards, Petar!
>> Karlovo, Bulgaria.
>> ---
>> Public PGP Key at:
>> http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x19658550C3110611
>> Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
>> For additional commands, e-mail: dev-help@maven.apache.org
>>
>>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: svn commit: r1695142 -
/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
Posted by he...@free.fr.
now that you told it, I'd seriously prefer change Doxia Markdown parser to use an XhtmlParser instance internally instead of extending XhtmlParser while completely replacing content parsed by the Xhtml parser: this would be a lot more clear (and would avoid adding bloat to getType())
I didn't really try, I don't know if this change is really complex
did you try?
Regards,
Hervé
----- Mail original -----
De: "Petar Tahchiev" <pa...@gmail.com>
À: "Maven Developers List" <de...@maven.apache.org>
Envoyé: Mercredi 12 Août 2015 08:39:12
Objet: Re: svn commit: r1695142 - /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
OK,
sorry I wasn't aware that user specifying an input encoding for xml file
would be considered as introducing a bug. Great for the test-case - I will
revert my changes and work for a fix in the MarkdownParser. Would
overriding the getType() method of the MarkdownParser be considered as a
valid solution?
2015-08-12 2:42 GMT+03:00 <he...@free.fr>:
> IIUC, your concerns are about Mardown: if Markdown parser has a bug, don't
> hesitate to fix it
> but do not break content for normal XML parsers, like fml or xdoc
>
> since your change did not make unit tests fail, this proves unit tests are
> too weak: I just improved them in r1695408 to fail (and show clearly what
> you are breaking)
> and I reopened DOXIASITETOOLS-104
>
> You're probably right that making Markdown parser *extend* XhtmlParser is
> probably wrong: it should *use* an XhtmlParser, but not extend it
>
> Regards,
>
> Hervé
>
> ----- Mail original -----
> De: "Petar Tahchiev" <pa...@gmail.com>
> À: "Maven Developers List" <de...@maven.apache.org>
> Envoyé: Mardi 11 Août 2015 11:36:28
> Objet: Re: svn commit: r1695142 -
> /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>
> Hi Herve,
>
> I did this because seems like a parser can be of type XML even if it's not
> parsing only XML - for example the MarkdownParser (which is in doxia and
> extends from the XmlParser) getType() returns 2 (XML parser type). I guess
> there are two ways to go here - 1) first would be to allow the user to
> force an encoding. It's his/hers decision and he/she takes the
> responsibility. 2) Would be to override the XmlParser:getType() method in
> MarkdownParser and make it return 0 (UNKNOWN_TYPE). To me this would lead
> to inconsistency, because the MarkdownParser extends from XmlParser, but
> returns another type. Furthermore I don't agree markdown syntax is in fact
> xml syntax.
>
>
> 2015-08-11 11:04 GMT+03:00 <he...@free.fr>:
>
> > wow, I don't like this
> > in XML, encoding is self provided
> >
> > with such feature, an XML-invalid document can be read by Maven (and
> Maven
> > only, since it is XML-invalid)
> >
> > I'm -1 on this: we can't help people make Maven-specific pseudo XML
> >
> > Regards,
> >
> > Hervé
> >
> > ----- Mail original -----
> > De: ptahchiev@apache.org
> > À: commits@maven.apache.org
> > Envoyé: Lundi 10 Août 2015 20:00:00
> > Objet: svn commit: r1695142 -
> >
> /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> >
> > Author: ptahchiev
> > Date: Mon Aug 10 18:00:00 2015
> > New Revision: 1695142
> >
> > URL: http://svn.apache.org/r1695142
> > Log:
> > Check for user's provided encoding, and only if it's null then use the
> > encoding of the xml document. Closes [DOXIASITETOOLS-104]
> >
> > Modified:
> >
> >
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> >
> > Modified:
> >
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> > URL:
> >
> http://svn.apache.org/viewvc/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java?rev=1695142&r1=1695141&r2=1695142&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> > (original)
> > +++
> >
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> > Mon Aug 10 18:00:00 2015
> > @@ -389,7 +389,14 @@ public class DefaultSiteRenderer
> > switch ( parser.getType() )
> > {
> > case Parser.XML_TYPE:
> > - reader = ReaderFactory.newXmlReader( doc );
> > + if ( siteContext.getInputEncoding() != null )
> > + {
> > + reader = ReaderFactory.newReader( doc,
> > siteContext.getInputEncoding() );
> > + }
> > + else
> > + {
> > + reader = ReaderFactory.newXmlReader( doc );
> > + }
> > if ( siteContext.isValidate() )
> > {
> > reader = validate( reader, resource );
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
> >
> >
>
>
> --
> Regards, Petar!
> Karlovo, Bulgaria.
> ---
> Public PGP Key at:
> http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x19658550C3110611
> Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
--
Regards, Petar!
Karlovo, Bulgaria.
---
Public PGP Key at:
http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x19658550C3110611
Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: svn commit: r1695142 - /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
Posted by Petar Tahchiev <pa...@gmail.com>.
OK,
sorry I wasn't aware that user specifying an input encoding for xml file
would be considered as introducing a bug. Great for the test-case - I will
revert my changes and work for a fix in the MarkdownParser. Would
overriding the getType() method of the MarkdownParser be considered as a
valid solution?
2015-08-12 2:42 GMT+03:00 <he...@free.fr>:
> IIUC, your concerns are about Mardown: if Markdown parser has a bug, don't
> hesitate to fix it
> but do not break content for normal XML parsers, like fml or xdoc
>
> since your change did not make unit tests fail, this proves unit tests are
> too weak: I just improved them in r1695408 to fail (and show clearly what
> you are breaking)
> and I reopened DOXIASITETOOLS-104
>
> You're probably right that making Markdown parser *extend* XhtmlParser is
> probably wrong: it should *use* an XhtmlParser, but not extend it
>
> Regards,
>
> Hervé
>
> ----- Mail original -----
> De: "Petar Tahchiev" <pa...@gmail.com>
> À: "Maven Developers List" <de...@maven.apache.org>
> Envoyé: Mardi 11 Août 2015 11:36:28
> Objet: Re: svn commit: r1695142 -
> /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>
> Hi Herve,
>
> I did this because seems like a parser can be of type XML even if it's not
> parsing only XML - for example the MarkdownParser (which is in doxia and
> extends from the XmlParser) getType() returns 2 (XML parser type). I guess
> there are two ways to go here - 1) first would be to allow the user to
> force an encoding. It's his/hers decision and he/she takes the
> responsibility. 2) Would be to override the XmlParser:getType() method in
> MarkdownParser and make it return 0 (UNKNOWN_TYPE). To me this would lead
> to inconsistency, because the MarkdownParser extends from XmlParser, but
> returns another type. Furthermore I don't agree markdown syntax is in fact
> xml syntax.
>
>
> 2015-08-11 11:04 GMT+03:00 <he...@free.fr>:
>
> > wow, I don't like this
> > in XML, encoding is self provided
> >
> > with such feature, an XML-invalid document can be read by Maven (and
> Maven
> > only, since it is XML-invalid)
> >
> > I'm -1 on this: we can't help people make Maven-specific pseudo XML
> >
> > Regards,
> >
> > Hervé
> >
> > ----- Mail original -----
> > De: ptahchiev@apache.org
> > À: commits@maven.apache.org
> > Envoyé: Lundi 10 Août 2015 20:00:00
> > Objet: svn commit: r1695142 -
> >
> /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> >
> > Author: ptahchiev
> > Date: Mon Aug 10 18:00:00 2015
> > New Revision: 1695142
> >
> > URL: http://svn.apache.org/r1695142
> > Log:
> > Check for user's provided encoding, and only if it's null then use the
> > encoding of the xml document. Closes [DOXIASITETOOLS-104]
> >
> > Modified:
> >
> >
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> >
> > Modified:
> >
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> > URL:
> >
> http://svn.apache.org/viewvc/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java?rev=1695142&r1=1695141&r2=1695142&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> > (original)
> > +++
> >
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> > Mon Aug 10 18:00:00 2015
> > @@ -389,7 +389,14 @@ public class DefaultSiteRenderer
> > switch ( parser.getType() )
> > {
> > case Parser.XML_TYPE:
> > - reader = ReaderFactory.newXmlReader( doc );
> > + if ( siteContext.getInputEncoding() != null )
> > + {
> > + reader = ReaderFactory.newReader( doc,
> > siteContext.getInputEncoding() );
> > + }
> > + else
> > + {
> > + reader = ReaderFactory.newXmlReader( doc );
> > + }
> > if ( siteContext.isValidate() )
> > {
> > reader = validate( reader, resource );
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> > For additional commands, e-mail: dev-help@maven.apache.org
> >
> >
>
>
> --
> Regards, Petar!
> Karlovo, Bulgaria.
> ---
> Public PGP Key at:
> http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x19658550C3110611
> Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
--
Regards, Petar!
Karlovo, Bulgaria.
---
Public PGP Key at:
http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x19658550C3110611
Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
Re: svn commit: r1695142 -
/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
Posted by he...@free.fr.
IIUC, your concerns are about Mardown: if Markdown parser has a bug, don't hesitate to fix it
but do not break content for normal XML parsers, like fml or xdoc
since your change did not make unit tests fail, this proves unit tests are too weak: I just improved them in r1695408 to fail (and show clearly what you are breaking)
and I reopened DOXIASITETOOLS-104
You're probably right that making Markdown parser *extend* XhtmlParser is probably wrong: it should *use* an XhtmlParser, but not extend it
Regards,
Hervé
----- Mail original -----
De: "Petar Tahchiev" <pa...@gmail.com>
À: "Maven Developers List" <de...@maven.apache.org>
Envoyé: Mardi 11 Août 2015 11:36:28
Objet: Re: svn commit: r1695142 - /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
Hi Herve,
I did this because seems like a parser can be of type XML even if it's not
parsing only XML - for example the MarkdownParser (which is in doxia and
extends from the XmlParser) getType() returns 2 (XML parser type). I guess
there are two ways to go here - 1) first would be to allow the user to
force an encoding. It's his/hers decision and he/she takes the
responsibility. 2) Would be to override the XmlParser:getType() method in
MarkdownParser and make it return 0 (UNKNOWN_TYPE). To me this would lead
to inconsistency, because the MarkdownParser extends from XmlParser, but
returns another type. Furthermore I don't agree markdown syntax is in fact
xml syntax.
2015-08-11 11:04 GMT+03:00 <he...@free.fr>:
> wow, I don't like this
> in XML, encoding is self provided
>
> with such feature, an XML-invalid document can be read by Maven (and Maven
> only, since it is XML-invalid)
>
> I'm -1 on this: we can't help people make Maven-specific pseudo XML
>
> Regards,
>
> Hervé
>
> ----- Mail original -----
> De: ptahchiev@apache.org
> À: commits@maven.apache.org
> Envoyé: Lundi 10 Août 2015 20:00:00
> Objet: svn commit: r1695142 -
> /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>
> Author: ptahchiev
> Date: Mon Aug 10 18:00:00 2015
> New Revision: 1695142
>
> URL: http://svn.apache.org/r1695142
> Log:
> Check for user's provided encoding, and only if it's null then use the
> encoding of the xml document. Closes [DOXIASITETOOLS-104]
>
> Modified:
>
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>
> Modified:
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> URL:
> http://svn.apache.org/viewvc/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java?rev=1695142&r1=1695141&r2=1695142&view=diff
>
> ==============================================================================
> ---
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> (original)
> +++
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> Mon Aug 10 18:00:00 2015
> @@ -389,7 +389,14 @@ public class DefaultSiteRenderer
> switch ( parser.getType() )
> {
> case Parser.XML_TYPE:
> - reader = ReaderFactory.newXmlReader( doc );
> + if ( siteContext.getInputEncoding() != null )
> + {
> + reader = ReaderFactory.newReader( doc,
> siteContext.getInputEncoding() );
> + }
> + else
> + {
> + reader = ReaderFactory.newXmlReader( doc );
> + }
> if ( siteContext.isValidate() )
> {
> reader = validate( reader, resource );
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
--
Regards, Petar!
Karlovo, Bulgaria.
---
Public PGP Key at:
http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x19658550C3110611
Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
For additional commands, e-mail: dev-help@maven.apache.org
Re: svn commit: r1695142 - /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
Posted by Petar Tahchiev <pa...@gmail.com>.
Hi Herve,
I did this because seems like a parser can be of type XML even if it's not
parsing only XML - for example the MarkdownParser (which is in doxia and
extends from the XmlParser) getType() returns 2 (XML parser type). I guess
there are two ways to go here - 1) first would be to allow the user to
force an encoding. It's his/hers decision and he/she takes the
responsibility. 2) Would be to override the XmlParser:getType() method in
MarkdownParser and make it return 0 (UNKNOWN_TYPE). To me this would lead
to inconsistency, because the MarkdownParser extends from XmlParser, but
returns another type. Furthermore I don't agree markdown syntax is in fact
xml syntax.
2015-08-11 11:04 GMT+03:00 <he...@free.fr>:
> wow, I don't like this
> in XML, encoding is self provided
>
> with such feature, an XML-invalid document can be read by Maven (and Maven
> only, since it is XML-invalid)
>
> I'm -1 on this: we can't help people make Maven-specific pseudo XML
>
> Regards,
>
> Hervé
>
> ----- Mail original -----
> De: ptahchiev@apache.org
> À: commits@maven.apache.org
> Envoyé: Lundi 10 Août 2015 20:00:00
> Objet: svn commit: r1695142 -
> /maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>
> Author: ptahchiev
> Date: Mon Aug 10 18:00:00 2015
> New Revision: 1695142
>
> URL: http://svn.apache.org/r1695142
> Log:
> Check for user's provided encoding, and only if it's null then use the
> encoding of the xml document. Closes [DOXIASITETOOLS-104]
>
> Modified:
>
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
>
> Modified:
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> URL:
> http://svn.apache.org/viewvc/maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java?rev=1695142&r1=1695141&r2=1695142&view=diff
>
> ==============================================================================
> ---
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> (original)
> +++
> maven/doxia/doxia-sitetools/trunk/doxia-site-renderer/src/main/java/org/apache/maven/doxia/siterenderer/DefaultSiteRenderer.java
> Mon Aug 10 18:00:00 2015
> @@ -389,7 +389,14 @@ public class DefaultSiteRenderer
> switch ( parser.getType() )
> {
> case Parser.XML_TYPE:
> - reader = ReaderFactory.newXmlReader( doc );
> + if ( siteContext.getInputEncoding() != null )
> + {
> + reader = ReaderFactory.newReader( doc,
> siteContext.getInputEncoding() );
> + }
> + else
> + {
> + reader = ReaderFactory.newXmlReader( doc );
> + }
> if ( siteContext.isValidate() )
> {
> reader = validate( reader, resource );
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@maven.apache.org
> For additional commands, e-mail: dev-help@maven.apache.org
>
>
--
Regards, Petar!
Karlovo, Bulgaria.
---
Public PGP Key at:
http://pgp.mit.edu:11371/pks/lookup?op=get&search=0x19658550C3110611
Key Fingerprint: A369 A7EE 61BC 93A3 CDFF 55A5 1965 8550 C311 0611