[Twisted-Python] Conch text attribute flattening quirks
Hello, I'm hoping to wrap up work on ticket #3844 [1] before the next release of Twisted, however there are a few quirks in the small DSL used for specifying text with attributes (see the ticket for some commentary on these.) I decided to look more closely at what happens with Conch (which is where the DSL originally came from and has remained largely unchanged) in some of the places I was having problems with the IRC formatting. I was struck by something that looks like it is rather incorrect behaviour (on Twisted trunk as of 2012/2/22 and Twisted 11.0.0 on Debian):
My expectations are that only " world " will be marked up with bold attributes, since it is the only piece of content in the "bold" attribute, while "hello" and "quux" both appear in normal text, i.e. without any additional markup. Looking at the output you can see that "hello" appears as normal text and then " world quux" appears in bold. Is this the expected behaviour? There are no tests that actually test this particular case, as far as I can tell, except for twisted.conch.test.test_text.EfficiencyTestCase but that is marked as "todo" (the tests themselves do indeed fail.) While the name of this test case indicates tests for efficient markup, as far as I can tell these tests also assume (and fail) that the structure is flattened the same way I was expecting. Any guidance would be appreciated. Regards [1] <http://twistedmatrix.com/trac/ticket/3844> -- Jonathan
On Wed, Feb 22, 2012 at 01:47:46PM +0200, Jonathan Jacobs wrote:
Since it's in twisted.conch, I'm guessing that this character-attribute stuff is designed to model the VT100 character attribute system, rather than some generic tree-of-strings-and-attributes. For example, somebody used to the way HTML works might want to nest bold and italics like this: <i>hello <b>world</b> quux</i> However, to achieve the same result on a traditional terminal (and using the tput(1) command to produce the formatting codes), you'd have to do something like this: tput sitm # enable italics echo -n "hello " tput bold # enable bold echo -n "world" tput sgr0 # disable all special attributes tput sitm # enable italics again echo " quux" ...that is, there's no code for 'end bold' or 'end italics' (or blink, dim, underline, invisible, any kind of colouring, etc.) just an 'end all special attributes' code. Therefore it's reasonable for conch's helper library to not handle nested formatting, since no terminal program will produce such a thing, because it's impossible to represent in the VT100/VT200 formatting language. I guess an argument could be made that the helper function should track which attributes are enabled at any particular point in the string, and calculate the correct sequence of disable-everything/re-enable-the- remaining-attributes codes, but evidently nobody's needed such a thing before.
On Wed, Feb 22, 2012 at 17:25, Tim Allen <screwtape@froup.com> wrote:
I understand this, but then what is the point of placing content into A.bold in the first place? Why not just drop A.bold into a flat stream: [foo, A.bold, bar] What's more confusing is that there are a handful of tests that describe this nesting and even test it, although none of them (except for the one marked TODO, which fails because of the problem I've described) actually test adding things to the parent level after nesting an attribute.
It's not actually even all that hard, there is nothing in particular to additionally track if you don't mind being a little inefficient about it, which is how the IRC code in #3844 does it. -- Jonathan
On 22 Feb, 04:15 pm, jonathan+twisted@jsphere.com wrote:
I think the behavior you observed is a bug in the implementation (I noticed it during our IRC conversation the other day as well, but wasn't sure if I'd made a mistake or misread something). It should be fixed, and perhaps that TODO test made to pass. :) Jean-Paul
On Thu, Feb 23, 2012 at 16:12, <exarkun@twistedmatrix.com> wrote:
Should I file a new ticket for this and do the work in a separate branch? In terms of making the TODO test pass: Unless I'm mistaken it looks to me that the render method (toVT102, toMIRCControlCode, etc.) will have to be passed the current attribute state in order to determine what it needs to emit to be most efficient. Do you see any better way to achieve this?
On Wed, Feb 22, 2012 at 01:47:46PM +0200, Jonathan Jacobs wrote:
Since it's in twisted.conch, I'm guessing that this character-attribute stuff is designed to model the VT100 character attribute system, rather than some generic tree-of-strings-and-attributes. For example, somebody used to the way HTML works might want to nest bold and italics like this: <i>hello <b>world</b> quux</i> However, to achieve the same result on a traditional terminal (and using the tput(1) command to produce the formatting codes), you'd have to do something like this: tput sitm # enable italics echo -n "hello " tput bold # enable bold echo -n "world" tput sgr0 # disable all special attributes tput sitm # enable italics again echo " quux" ...that is, there's no code for 'end bold' or 'end italics' (or blink, dim, underline, invisible, any kind of colouring, etc.) just an 'end all special attributes' code. Therefore it's reasonable for conch's helper library to not handle nested formatting, since no terminal program will produce such a thing, because it's impossible to represent in the VT100/VT200 formatting language. I guess an argument could be made that the helper function should track which attributes are enabled at any particular point in the string, and calculate the correct sequence of disable-everything/re-enable-the- remaining-attributes codes, but evidently nobody's needed such a thing before.
On Wed, Feb 22, 2012 at 17:25, Tim Allen <screwtape@froup.com> wrote:
I understand this, but then what is the point of placing content into A.bold in the first place? Why not just drop A.bold into a flat stream: [foo, A.bold, bar] What's more confusing is that there are a handful of tests that describe this nesting and even test it, although none of them (except for the one marked TODO, which fails because of the problem I've described) actually test adding things to the parent level after nesting an attribute.
It's not actually even all that hard, there is nothing in particular to additionally track if you don't mind being a little inefficient about it, which is how the IRC code in #3844 does it. -- Jonathan
On 22 Feb, 04:15 pm, jonathan+twisted@jsphere.com wrote:
I think the behavior you observed is a bug in the implementation (I noticed it during our IRC conversation the other day as well, but wasn't sure if I'd made a mistake or misread something). It should be fixed, and perhaps that TODO test made to pass. :) Jean-Paul
On Thu, Feb 23, 2012 at 16:12, <exarkun@twistedmatrix.com> wrote:
Should I file a new ticket for this and do the work in a separate branch? In terms of making the TODO test pass: Unless I'm mistaken it looks to me that the render method (toVT102, toMIRCControlCode, etc.) will have to be passed the current attribute state in order to determine what it needs to emit to be most efficient. Do you see any better way to achieve this?
participants (3)
-
exarkun＠twistedmatrix.com -
Jonathan Jacobs -
Tim Allen