Default to TERM=dumb even if $TERM is set but empty

[Whissi]

We run into this because a Gentoo tinderbox system trying to build current Mozilla Firefox failed to do so because TERM wasn't set. Well, it was set but it was set to an empty value, e.g. TERM=.

This small addition to #137 will fix such scenarios as well.

In theory a user could still have set TERM=" " which will cause troubles. Not sure if you want to do something like

termEnv = environ.get('TERM', 'dumb')
if not termEnv.strip():
    termEnv = 'dumb'

setupterm(kind or termEnv,

to catch almost all scenarios...

[jgkamat]

We can't handle this case properly in this way, for example, TERM could be set to I am never going to be a valid terminfo database file!.

[jay@laythe blessings]$ TERM='I am never going to be a valid terminfo database file!' py -c 'import blessings; t = blessings.Terminal(); print(blessings.tigetstr("sgr0"))'       [jay/term-crash↓] [11652]  7:47PM
zsh: can't find terminal definition for I am never going to be a valid terminfo database file!
Traceback (most recent call last):
  File "<string>", line 1, in <module>
_curses.error: must call (at least) setupterm() first

I'm relying on setupterm throwing an error in these cases, and turning off styling.

I'm honestly baffled where blessings.tigetstr is defined. I'm guessing blessings.tigetstr is actually the same as curses.tigetstr. I think that once does_styling is False, we should really stop making any curses calls (as we may not have curses initialized), and if someone wants to use curses.tigetstr, that's their responsibility.

However, if we wanted to be a bit more robust, we could ensure that setupterm is always called with dumb, although I'm worried this will be a little too invasive. I don't think dumb is in posix, so we can't rely on this. Maybe this would work?

diff --git a/blessings/__init__.py b/blessings/__init__.py
index fdceb09..0ee645c 100644
--- a/blessings/__init__.py
+++ b/blessings/__init__.py
@@ -100,7 +100,12 @@ class Terminal(object):
             except curses.error:
                 # There was an error setting up the terminal, either curses is
                 # not supported or TERM is incorrectly set. Fall back to dumb.
-                self._does_styling = False
+                try:
+                    setupterm('dumb', self._init_descriptor)
+                except curses.error:
+                    # We don't even have dumb available, turn of styling and
+                    # hope for the best.
+                    self._does_styling = False
 
         self.stream = stream

[Whissi]

I understand. Your proposed solution would work for us.

But don't we know that

# We don't even have dumb available, turn of styling and
# hope for the best.

doesn't work? I guess that's what you meant with

I'm honestly baffled where blessings.tigetstr is defined. [...] we should really stop making any curses calls

because if "dumb" isn't available (that's like before when we called "" (empty) or your "I am never going to be a valid terminfo database file" value), we would run into the error I have shown in #137 I think or am I wrong?

[jgkamat]

because if "dumb" isn't available (that's like before when we called "" (empty) or your "I am never going to be a valid terminfo database file" value), we would run into the error I have shown in #137 I think or am I wrong?

Yes, you would. The thing I don't understand is that the line that's erroring is:

self._sgr0 = blessings.tigetstr('sgr0') or '' if terminal and blessings else ''

and I cannot find the definition of blessings.tigetstr anywhere. Because of that, I'm guessing that somehow, blessings.tigetstr is the same as curses.tigetstr. If someone calls curses.tigetstr without setting up curses (as we do), there's nothing we can do to stop that error as they aren't really using blessings. If they use functions that are part of the Terminal object and get that crash, that is more actionable (unless I'm just blanking on where the definition for that is).

Maybe we should define a blessings.tigetstr which just returns curses.tigetstr if styling is supported, and None otherwise. It would help the rest of blessings be "more correct" as blessings makes a bunch of calls to tigetstr. Since I didn't test all the codepaths, it's very possible one of these calls is happening when does_styling is False (and causing a crash in that case).

In practice though 99.9% of machines probably have dumb, and the 0.1% of users who don't have it set probably know what they are doing or have similar problems (and set TERM correctly).

[Arfrever]

Definition of blessings.tigetstr is obviously in blessings/__init__.py in line 6:

from curses import setupterm, tigetnum, tigetstr, tparm

[jgkamat]

Indeed, but then that's not something defined in blessings at all. That's curses.tigetstr, not blessings.tigetstr. Because of that, we can't fix it, that's just an improper use of curses.tigetstr.

If blessings changes it's usages of tigetstr to curses.tigetstr instead of importing it with from (which it can do, as that's not part of it's API), that code will break.

[jgkamat]

I've submitted a small patch to firefox to fix the usage of tigetstr in the blessings namespace, which I think should also resolve the error @Whissi was seeing (once they pull from blessings).

[erikrose]

Do you see anything wrong with setupterm(kind or environ.get('TERM') or 'dumb',, just to remove repetition?

[Whissi]

Not sure if you are asking me. No, I don't see anything wrong with that. The idea is:

Use "kind", if not set, try to read environment variable "TERM" and treat an empty value like TERM is set to "dumb".

[erikrose]

Thanks! Tweaked and merged.