Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: Release 1.1
    • Fix Version/s: 1.2
    • Component/s: Server
    • Labels:
      None

      Description

      Try the following messages:

      Does this work?

      How about this?

      In the first message, "this" is rendered in bold with no asterisks. In the second message, it is rendered in normal text as "this".

      The first behavior is correct.

        Activity

        Hide
        Hudson added a comment -

        Integrated in ESME #509 (See https://hudson.apache.org/hudson/job/ESME/509/)
        ESME-307 Inline markup is terminated properly by punctuation and does not generate an extra space.

        Show
        Hudson added a comment - Integrated in ESME #509 (See https://hudson.apache.org/hudson/job/ESME/509/ ) ESME-307 Inline markup is terminated properly by punctuation and does not generate an extra space.
        Hide
        Vassil Dichev added a comment -

        Inline markup (bold and italic) now terminates correctly on either punctuation or space, and doesn't generate an extra space in the resulting HTML. Considering that Google Talk doesn't define almost any punctuation preceding the inline markup, this setup should be good enough for a simple parser like this.

        Show
        Vassil Dichev added a comment - Inline markup (bold and italic) now terminates correctly on either punctuation or space, and doesn't generate an extra space in the resulting HTML. Considering that Google Talk doesn't define almost any punctuation preceding the inline markup, this setup should be good enough for a simple parser like this.
        Hide
        Vassil Dichev added a comment -

        After carefully reviewing the markup for reStructuredText, Textile and Google Talk, I've come to the conclusion that characters which precede inline markup are not symmetric to characters which follow it. To illustrate:

        1. Google Talk:
        preceding:
        -_
        following:
        !.,;:?-_

        2. Textile:
        preceding:
        .,\"'?!;:
        following:
        !"#$%&'()*+,-./:;<=>?@[]^_`

        {|}

        ~

        3. reSructucedText:
        preceding:
        '"([

        {<-/: following: '")]}

        >-/:.,;!?\

        I've initially tried to model ESME's simplified markup after Google Talk. Considering it hardly has any characters preceding inline markup, I'm OK with having none for ESME and letting markup end with any punctuation.

        One way to solve the trailing space problem is to make punctuation a part of the inline markup, in which case it will appear before the closing space. What do you think, should I commit my current implementation?

        In the meantime, I will experiment with a couple of different variants.

        Show
        Vassil Dichev added a comment - After carefully reviewing the markup for reStructuredText, Textile and Google Talk, I've come to the conclusion that characters which precede inline markup are not symmetric to characters which follow it. To illustrate: 1. Google Talk: preceding: -_ following: !.,;:?-_ 2. Textile: preceding: .,\"'?!;: following: !"#$%&'()*+,-./:;<=>?@[]^_` {|} ~ 3. reSructucedText: preceding: '"([ {<-/: following: '")]} >-/:.,;!?\ I've initially tried to model ESME's simplified markup after Google Talk. Considering it hardly has any characters preceding inline markup, I'm OK with having none for ESME and letting markup end with any punctuation. One way to solve the trailing space problem is to make punctuation a part of the inline markup, in which case it will appear before the closing space. What do you think, should I commit my current implementation? In the meantime, I will experiment with a couple of different variants.
        Hide
        Ethan Jewett added a comment -

        Hmmm, I see what you are saying Vassil. In the meantime, any ideas for getting rid of the trailing space that is currently inserted?

        Show
        Ethan Jewett added a comment - Hmmm, I see what you are saying Vassil. In the meantime, any ideas for getting rid of the trailing space that is currently inserted?
        Hide
        Vassil Dichev added a comment -

        Including Lift's Textile parser will take more time and resources though. We need to find a way to integrate our own hashtags and username parsing with the textile markup, and that's not a trivial task. In addition, even if we used Lift's Textile parser, this bug would still be open so certainly we can't fix it before the release using Lift.

        Show
        Vassil Dichev added a comment - Including Lift's Textile parser will take more time and resources though. We need to find a way to integrate our own hashtags and username parsing with the textile markup, and that's not a trivial task. In addition, even if we used Lift's Textile parser, this bug would still be open so certainly we can't fix it before the release using Lift.
        Hide
        Vladimir Ivanov added a comment -

        I completely agree, including Lift's TextileParser (with possible fixed inline markup punctuation) into ESME would be much more elegant solution.

        Thanks Vassil and Ethan!

        Show
        Vladimir Ivanov added a comment - I completely agree, including Lift's TextileParser (with possible fixed inline markup punctuation) into ESME would be much more elegant solution. Thanks Vassil and Ethan!
        Hide
        Ethan Jewett added a comment -

        The current patch does seem to parse punctuation in the middle of a line OK, but it inserts a space in the message at that point (or maybe it is a newline?).

        I'm wondering if it might make sense to switch to the Lift Textile parser and then bring up this issue on the Lift list. Would it make sense to make the switch?

        I'm also thinking we should just push this issue to the next release, but let's work on it until Dick is back and then if this is still the only blocking issue we can push it off one release.

        Show
        Ethan Jewett added a comment - The current patch does seem to parse punctuation in the middle of a line OK, but it inserts a space in the message at that point (or maybe it is a newline?). I'm wondering if it might make sense to switch to the Lift Textile parser and then bring up this issue on the Lift list. Would it make sense to make the switch? I'm also thinking we should just push this issue to the next release, but let's work on it until Dick is back and then if this is still the only blocking issue we can push it off one release.
        Hide
        Vassil Dichev added a comment -

        ESME's parser is a much-simplified version of Lift's TextileParser, and after checking Lift's behaves the same way with inline markup- punctuation doesn't end the markup:

        http://scala-tools.org/mvnsites/liftweb-2.2/framework/scaladocs/lift-modules/lift-textile/src/main/scala/net/liftweb/textile/TextileParser.scala.html

        This might be an omission in the parser considering that the de-facto standard implementation terminates inline markup correctly on punctuation:

        http://textile.thresholdstate.com/

        Since I still have plans to include textile in ESME again, I might consider fixing it in Lift after we solve it in ESME.

        Show
        Vassil Dichev added a comment - ESME's parser is a much-simplified version of Lift's TextileParser, and after checking Lift's behaves the same way with inline markup- punctuation doesn't end the markup: http://scala-tools.org/mvnsites/liftweb-2.2/framework/scaladocs/lift-modules/lift-textile/src/main/scala/net/liftweb/textile/TextileParser.scala.html This might be an omission in the parser considering that the de-facto standard implementation terminates inline markup correctly on punctuation: http://textile.thresholdstate.com/ Since I still have plans to include textile in ESME again, I might consider fixing it in Lift after we solve it in ESME.
        Hide
        Vassil Dichev added a comment -

        I'm not sure adding punctuatioon to the spaceOrEnd parser is the right path yet. How would would punctuation which is not the end of the text parse then:

        how would?this parse

        If we decide that punctuation should end the special markup, then it should also begin next inline markup, and you have to modify begOrSpace too:

        would this?parse correctly

        Also, rather than being inconsistent and wondering what is punctuation, I'd rather use the regular expression Pattern.compile("""\p

        {Punct}

        """)

        It's also very informative to read the grammar rules for a couple of existing lightweight markup parsers, for instance the reStructuredText markup specification:

        http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#inline-markup

        Show
        Vassil Dichev added a comment - I'm not sure adding punctuatioon to the spaceOrEnd parser is the right path yet. How would would punctuation which is not the end of the text parse then: how would ?this parse If we decide that punctuation should end the special markup, then it should also begin next inline markup, and you have to modify begOrSpace too: would this? parse correctly Also, rather than being inconsistent and wondering what is punctuation, I'd rather use the regular expression Pattern.compile("""\p {Punct} """) It's also very informative to read the grammar rules for a couple of existing lightweight markup parsers, for instance the reStructuredText markup specification: http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#inline-markup
        Hide
        Hudson added a comment -

        Integrated in ESME #502 (See https://hudson.apache.org/hudson/job/ESME/502/)
        ESME-307 Added other punctuation to parser. Unfortunately there is still a problem with inserting an extra space after the bolded text.

        Show
        Hudson added a comment - Integrated in ESME #502 (See https://hudson.apache.org/hudson/job/ESME/502/ ) ESME-307 Added other punctuation to parser. Unfortunately there is still a problem with inserting an extra space after the bolded text.
        Hide
        Hudson added a comment -

        Integrated in ESME #500 (See https://hudson.apache.org/hudson/job/ESME/500/)
        ESME-307 Format rendering gets confused by some punctuation
        Idea from Vladimir Ivanov

        Show
        Hudson added a comment - Integrated in ESME #500 (See https://hudson.apache.org/hudson/job/ESME/500/ ) ESME-307 Format rendering gets confused by some punctuation Idea from Vladimir Ivanov
        Hide
        Ethan Jewett added a comment -

        Yup, I saw that, but the ticket got closed anyway and I wanted to give some explanation for why I was reopening it

        Show
        Ethan Jewett added a comment - Yup, I saw that, but the ticket got closed anyway and I wanted to give some explanation for why I was reopening it
        Hide
        Vladimir Ivanov added a comment -

        I agree. I wrote that in the comment.

        Show
        Vladimir Ivanov added a comment - I agree. I wrote that in the comment.
        Hide
        Ethan Jewett added a comment -

        We need to make this fix more complete. The patch deals with "?", but we also need to handle at least "!" and ".", and probably ",", ":", ";", and "-". Is there a generic representation of punctuation? That might do the trick.

        Show
        Ethan Jewett added a comment - We need to make this fix more complete. The patch deals with "?", but we also need to handle at least "!" and ".", and probably ",", ":", ";", and "-". Is there a generic representation of punctuation? That might do the trick.
        Hide
        Dick Hirsch added a comment -

        Thanks
        Checked locally and the solution works

        Show
        Dick Hirsch added a comment - Thanks Checked locally and the solution works
        Hide
        Vladimir Ivanov added a comment -

        It seems the problem relates to 'spaceOrEnd' parser. As it's name suggests Space or EOL symbol is expected at the end of string to be parsed. But in case of 'How about this?' string, '?' symbol finishes the line, so it isn't parsed as expected. When I add, for example rep1('?') expression to spaceOrEnd grammar like this:

        lazy val spaceOrEnd: Parser[Int] = EOL ^^^ 0 | (rep1(' ') | rep1('?')) ^^

        {case lst => lst.length}

        text becomes highlighted as expected. So special symbols, like '?', '!' etc should be clearly defined and be carefully applied to this case (to avoid braking another cases where 'spaceOrEnd' parser is used).

        Show
        Vladimir Ivanov added a comment - It seems the problem relates to 'spaceOrEnd' parser. As it's name suggests Space or EOL symbol is expected at the end of string to be parsed. But in case of 'How about this ?' string, '?' symbol finishes the line, so it isn't parsed as expected. When I add, for example rep1('?') expression to spaceOrEnd grammar like this: lazy val spaceOrEnd: Parser [Int] = EOL ^^^ 0 | (rep1(' ') | rep1('?')) ^^ {case lst => lst.length} text becomes highlighted as expected. So special symbols, like '?', '!' etc should be clearly defined and be carefully applied to this case (to avoid braking another cases where 'spaceOrEnd' parser is used).
        Hide
        Dick Hirsch added a comment -

        My research shows that the problem is here:

        bold - MsgParser.scala

        def surrounded(sep: String)(constructor: (String) => MsgInfo): Parser[MsgInfo] =
        begOrSpace ~ accept(sep) > rep1(not(spaceEOF) ~ not(accept(sep)) ~ not(hashTag) ~ not(atName) ~> anyChar) < accept(sep) ~ peek(spaceOrEnd) ^^

        { case xs => constructor(xs.mkString) }
        Show
        Dick Hirsch added a comment - My research shows that the problem is here: bold - MsgParser.scala def surrounded(sep: String)(constructor: (String) => MsgInfo): Parser [MsgInfo] = begOrSpace ~ accept(sep) > rep1(not(spaceEOF) ~ not(accept(sep)) ~ not(hashTag) ~ not(atName) ~> anyChar) < accept(sep) ~ peek(spaceOrEnd) ^^ { case xs => constructor(xs.mkString) }

          People

          • Assignee:
            Vassil Dichev
            Reporter:
            Ethan Jewett
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development