search

pazz / alot

Terminal-based Mail User Agent
GNU General Public License v3.0
698 stars 164 forks source link

Not specifying editor_cmd results in undecipherable log #1234

Open javiertury opened 6 years ago

javiertury commented 6 years ago

If a user hasn't specified an editor in the config file with the option editor_cmd, alot fails to compose a message using the :compose command. The error happens just before the editor should be launched.

I have no editor specified in my system, $EDITOR is blank, but I have vim, vi, emacs, mu... from the distribution packages and are reachable from $PATH.

Python version: 2.7.14 Notmuch version: 0.25 Alot version: 0.6 and master a few days ago

The following log documents the error. Bear in mind that this is the with -d debug -l log, without debugging the output the user is clueless about the cause of the error.

DEBUG:globals:SUBJECT: "lsadjkf"
DEBUG:globals:No encryption by default, encrypt_by_default=none
INFO:envelope:editable headers: set(['To', 'From', 'Subject'])
DEBUG:globals:using editor_cmd: None
ERROR:ui:Traceback (most recent call last):
  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 317, in addCallback
    callbackKeywords=kw)
  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 306, in addCallbacks
    self._runCallbacks()
  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 587, in _runCallbacks
    current.result = callback(current.result, *args, **kw)
  File "/usr/lib/python2.7/site-packages/alot/ui.py", line 679, in call_apply
    return defer.maybeDeferred(cmd.apply, self)
--- <exception caught here> ---
  File "/usr/lib64/python2.7/site-packages/twisted/internet/defer.py", line 149, in maybeDeferred
    result = f(*args, **kw)
  File "/usr/lib/python2.7/site-packages/alot/commands/envelope.py", line 396, in apply
    refocus=self.refocus)
  File "/usr/lib/python2.7/site-packages/alot/commands/globals.py", line 330, in __init__
    if '%s' in editor_cmdstring:
exceptions.TypeError: argument of type 'NoneType' is not iterable

I believe alot should select a default editor from the available binaries in $PATH like vim, emacs, nano... and warn the user that there was no editor configured. It should not throwing an exception with uninformative or confusing information. When I first encountered the problem I thought it was because of missing or faulty dependencies instead of misconfiguration.

pazz commented 6 years ago

Yes I agree. This should not be so difficult, but somewhat tricky to get right. What should the order fallback be? alot config > $EDITOR > /bin/editor > Some fixed order over what we find in /bin/ ?

javiertury commented 6 years ago

I agree with the order. But now I realize the fallback chain get's increasingly complex after $EDITOR and /bin/editor.

Maybe it's enough to reduce the chain to alot config > $EDITOR. If none of the previous variables is set, display an error message in which the user is told to set the variable editor_cmd or $EDITOR. Afterwards, cancel the compose command.

lucc commented 6 years ago

alot config > $EDITOR > Error sound good to me.