Fix NullCallableString() when called with no or multiple arguments - #31

This behavior occurs because stdout is not a terminal ..

Old behavior:

$ python2.7 -c 'from blessings import Terminal; term = Terminal(); print(term.move(1, 2))' > /tmp/z && cat /tmp/z
Traceback (most recent call last):
File "", line 1, in
TypeError: call() takes exactly 2 arguments (3 given)
$ python2.7 -c 'from blessings import Terminal; term = Terminal(); print(term.clear())' > /tmp/z && cat /tmp/z
Traceback (most recent call last):
File "", line 1, in
TypeError: call() takes exactly 2 arguments (1 given)
$ python2.7 -c 'from blessings import Terminal; term = Terminal(); print(term.bold("woah dudes"))' > /tmp/z && cat /tmp/z
woah dudes

With vitek's patch:

$ python2.7 -c 'from blessings import Terminal; term = Terminal(); print(term.move(1, 2))' > /tmp/z && cat /tmp/z

$ python2.7 -c 'from blessings import Terminal; term = Terminal(); print(term.clear())' > /tmp/z && cat /tmp/z

$ python2.7 -c 'from blessings import Terminal; term = Terminal(); print(term.bold("woah dudes"))' > /tmp/z && cat /tmp/z
woah dudes

So it seems fine to me. I guess the only place this would fail if multiple arguments were passed where the first argument is non-int. I don't think there is any such use-case. A test case ala the above would be kind.

This behavior occurs because stdout is not a terminal ..

Sure, but blessings is supposed to work with a pipe as well as tty.

So it seems fine to me. I guess the only place this would fail if multiple arguments were passed where the first argument is non-int. I don't think there is any such use-case.

Not exactly, if it's called with noargs or multiple args it returns empty string. If it's called with one argument the logic is the same: return empty string on integer, argument the same otherwise.

A test case ala the above would be kind.

Ok, I'll do it.

I've added testcase.

thats cool. maybe erik is not so busy doing real work that he can accept it :-) what i mean by the use of *args and then returning arg[0] explicitly is for whether TypeError is thrown, they are not in the case of multiple string arguments as they normally would be when a Terminal's _does_styling is True:

$ python2.7 -c 'from blessings import Terminal; term = Terminal(); print(term.bold("1"))' > /tmp/z && cat /tmp/z
1
$ python2.7 -c 'from blessings import Terminal; term = Terminal(); print(term.bold("1", "2"))' > /tmp/z && cat /tmp/z

$ python2.7 -c 'from blessings import Terminal; term = Terminal(); print(term.bold("1", "2"))'
Traceback (most recent call last):
File "", line 1, in
TypeError: call() takes exactly 2 arguments (3 given)

perhaps you could retain *args, but throw a TypeError manually if arg[0] is non-int and len(args) is greater than 1 ... or not, its a very trivial difference :-)

Actually it's hard to catch. I don't like idea of raising exception it's a hack since ParametrizingString fully relies on tparm():

    def __call__(self, *args):
        try:
            # Re-encode the cap, because tparm() takes a bytestring in Python
            # 3. However, appear to be a plain Unicode string otherwise so
            # concats work.
            parametrized = tparm(self.encode('utf-8'), *args).decode('utf-8')
            return (parametrized if self._normal is None else
                    FormattingString(parametrized, self._normal))
        except curses.error:
            # Catch "must call (at least) setupterm() first" errors, as when
            # running simply `nosetests` (without progressive) on nose-
            # progressive. Perhaps the terminal has gone away between calling
            # tigetstr and calling tparm.
            return u''
        except TypeError:
            # If the first non-int (i.e. incorrect) arg was a string, suggest
            # something intelligent:
            if len(args) == 1 and isinstance(args[0], basestring):
                raise TypeError(
                    'A native or nonexistent capability template received '
                    '%r when it was expecting ints. You probably misspelled a '
                    'formatting call like bright_red_on_white(...).' % args)
            else:
                # Somebody passed a non-string; I don't feel confident
                # guessing what they were trying to do.
                raise

But may be you're right, it also raises TypeError under similar circumstances. Let's wait for Erik to make the final decision)

Hi, guys! I'm following. I'll have a look in a day or two.

Loading this back into my head now. It looks like only one of the assertions in your test was actually broken: eq_(t.move(1, 2), ''). The rest pass for me without any changes (and, in fact, the bold one occurs almost verbatim in test_formatting_functions_without_tty()), so I'll pare the testcase down.

Okay, loaded the last 4 decades of terminal legacy into my head. Geesh. 3 hours loading, 1 hour coding. :-)

I think for now we'll take the inexpensive route and not raise TypeError. The ultimate solution, which I point to in a new TODO, is to retire the dual-purpose NullCallableString in favor of two new classes, NullFormattingString and NullParametrizingString, which know how many params they take and can thus raise more clueful error messages. However, we'll need to add lots of squirrely little branches to all the formatting resolvers in Terminal to decide which of those to instantiate and return—a more ambitious endeavor for another day. Patched patch to come…

Okay, merged! If you two want to have a look, go for it, but I barely changed anything. Thanks for the good patch, @vitek, and for the clueful-as-always review comments and reasoning, @jquast. Someday before Python is no longer used as a language, I will get to your input patch. :-)

Looks like we have a fair number of minor fixes ready to go out. I'll probably let this bake in for a day and then cut a release.

Looks like we have a fair number of minor fixes ready to go out. I'll probably let this bake in for a day and then cut a release.

Yeah, that's a good idea)

About my PR: you're right my it helps with move() only cause I don't have to parametrize clear() call, that's my fault. Otoh it's better to raise an exception when move() is callled with no tty attached since it may lead to broken output. And the user must insert explicit check for is_a_tty.

The practical part of me agrees with you about making move throw exceptions when there's no tty. It's obviously a better user experience. The idealistic part of me disagrees, because it breaks blessings' delegation assumptions—which also have practical consequences. Please forgive me if I get philosophical for a minute; I don't think I've ever written any of this down, but we really should have it documented.

Blessings is, as its description string says, a thin wrapper around terminfo. That gives it a lot of flexibility: for example, @jquast was able to implement a sort of terminal server, resolving capabilities to strings remotely and then sending the bytestrings down to the client. If the server looked at move and got '' because it was running detached, he wouldn't have been able to do that. Keeping it thin also keeps it short and, after a fashion, simple.

However, I am constantly haunted by the fact that there's a demand for a higher abstraction layer. For example, it'd be great to support Windows, but there's just no terminal there to layer upon; we'd have to limit the API to higher-level primitives like location() and color(), eschewing things like hide_cursor and more niche caps I don't even anticipate. That's not a showstopper for me, but here's what is: blessings' API is innately tied to formatting codes being multiplexed into the same channel as text, e.g. print term.bold('thing'). You can even store term.bold in a variable and concatenate or output it later. That's not going to fly on Windows at all, or at least we'd have to invent our own string subclass that understands how to translate formatting tokens into Windows API calls, or we'd have to outlaw the use of "print" and substitute our own wrapper routine. This rules out s another blessings feature that I like: the ability to mix and match it with other terminal-formatting libraries. Since it doesn't keep much state, you can migrate a project to blessings gradually, or you can use blessings for low-level stuff and a higher-level kit like urwid for drawing windows and fields.

In short, I think that higher abstraction layer is where an exception-throwing move() belongs. Blessings doesn't know what move means; it just delegates to terminfo. Now, I could hear an argument that location() should throw an exception, but I'd be reticent on that as well, since, although Blessings knows the semantics of location, that distinction is not clear to the user: since both are just attributes of Terminal, a difference in behavior would be surprising.

So I think, for now, we'll leave it to the user to check is_a_tty, since the user knows both the semantics of the caps he's using and how important it is to his use case that they actually work. Now, I could hear an argument that is_a_tty should take force_styling into account. That is something we should think about, IMO.

Thanks for being tolerant of my philosophizing! I hope I haven't scared you off, as your patches are significantly improving blessings, and I've been enjoying our exchanges. Even this monologue has helped clarify to me what out future directions should be. Cheers!

Actually jquast would have been able to do his thing either way: move would have returned the right thing, since he told Terminal explicitly which term type to use.