Crash trying to set `srv.path.complete` to `off`

lenormf commented 4 years ago

Reproducer:

:set srv.path.complete off

Trace:

Traceback (most recent call last):
  File "/usr/bin/stig", line 11, in <module>
    load_entry_point('stig==0.11.0a0', 'console_scripts', 'stig')()
  File "/usr/lib/python3.8/site-packages/stig/__init__.py", line 25, in run
    main.run()
  File "/usr/lib/python3.8/site-packages/stig/main.py", line 102, in run
    if not tui.run(run_commands):
  File "/usr/lib/python3.8/site-packages/stig/tui/main.py", line 64, in run
    asyncio.get_event_loop().run_until_complete(objects.srvapi.stop_polling())
  File "/usr/lib/python3.8/asyncio/base_events.py", line 616, in run_until_complete
    return future.result()
  File "/usr/lib/python3.8/site-packages/stig/client/api.py", line 209, in stop_polling
    await poller.stop()
  File "/usr/lib/python3.8/site-packages/stig/client/aiotransmission/api_freespace.py", line 121, in stop
    await self._watcher_complete.stop(*args, **kwargs)
  File "/usr/lib/python3.8/site-packages/stig/client/aiotransmission/api_freespace.py", line 36, in stop
    await self._poller_size.stop(*args, **kwargs)
  File "/usr/lib/python3.8/site-packages/stig/client/poll.py", line 158, in stop
    await asyncio.wait_for(self._poll_loop_task, timeout=0)
  File "/usr/lib/python3.8/asyncio/tasks.py", line 461, in wait_for
    return fut.result()
  File "/usr/lib/python3.8/site-packages/stig/tui/main.py", line 60, in run
    tuiobjects.urwidloop.run()
  File "/usr/lib/python3.8/site-packages/urwid/main_loop.py", line 287, in run
    self._run()
  File "/usr/lib/python3.8/site-packages/urwid/main_loop.py", line 385, in _run
    self.event_loop.run()
  File "/usr/lib/python3.8/site-packages/stig/tui/urwidpatches.py", line 175, in run
    raise self._exc_info
  File "/usr/lib/python3.8/asyncio/events.py", line 81, in _run
    self._context.run(self._callback, *self._args)
  File "/usr/lib/python3.8/site-packages/stig/client/poll.py", line 77, in reraise
    task.result()
  File "/usr/lib/python3.8/site-packages/stig/client/poll.py", line 88, in _poll_loop
    await self._poll_task
  File "/usr/lib/python3.8/site-packages/stig/client/poll.py", line 123, in _do_poll
    self._run_callbacks(error=e)
  File "/usr/lib/python3.8/site-packages/stig/client/poll.py", line 138, in _run_callbacks
    self._on_error.send(error)
  File "/usr/lib/python3.8/site-packages/blinker/base.py", line 266, in send
    return [(receiver, receiver(sender, **kwargs))
  File "/usr/lib/python3.8/site-packages/blinker/base.py", line 266, in <listcomp>
    return [(receiver, receiver(sender, **kwargs))
  File "/usr/lib/python3.8/site-packages/stig/client/aiotransmission/api_freespace.py", line 96, in _handle_size_error
    raise error
  File "/usr/lib/python3.8/site-packages/stig/client/poll.py", line 117, in _do_poll
    response = await self._request()
  File "/usr/lib/python3.8/site-packages/stig/client/aiotransmission/rpc.py", line 462, in request
    return await self._send_request(rpc_request)
  File "/usr/lib/python3.8/site-packages/stig/client/aiotransmission/rpc.py", line 427, in _send_request
    raise RPCError(answer['result'].capitalize())
stig.client.errors.RPCError: Invalid RPC response: No such file or directory

HTH.

rndusr commented 4 years ago

This happens because stig interprets "off" as a path relative to the current value of "srv.path.complete", e.g. "/my/torrents/foo", which doesn't exist and the "rpc.free_space()" request reports the error.

Should be fixed more or less. Now it just hides the free space display for any path that doesn't exist. I'll try to find a good way report the error instead.

rndusr commented 4 years ago

The "No such file or directory" is now displayed next to the directory name.

It may get tight depending on terminal size, but I think it's better than ignoring the error.

lenormf commented 4 years ago

It feels a bit counter-intuitive. Setting srv.path.incomplete = off disables the feature, but srv.path.complete = off interprets the argument as a directory.

I think the intent is clear, in that case, that the user is trying to disable a setting that is not optional, so I would rather have a warning displayed saying something along the lines of "pass a fullpath to the off directory instead of using a keyword by itself".

The commit that was pushed to fix this issue is useful for paths that don't exist though, in any case.

lenormf commented 4 years ago

By the way, could you mention the issue number in the commit that closes it? Append "Fixes #1234" to the commit message, and Github will close the issue automatically, plus it allows users to jump to the patch directly from the ticket.

Thanks!

rndusr commented 4 years ago

I don't think the intent is clear. To me, "disabling the download path" doesn't make any sense. What did you expect when you set srv.path.complete to off?

I don't really want to make values illegal for no good reason. I justified making boolean values illegal for srv.path.incomplete by combining two settings into one (srv.path.incomplete and srv.path.incomplete.enabled). But I don't see any reason to make boolean values illegal for srv.path.complete.

I'll try to remember adding issue numbers to commit messages.

rndusr / stig

Crash trying to set `srv.path.complete` to `off` #161