ArduPilot / ArduPilot/MAVProxy

mavproxy_log: tab-complete on download

Open
#1,706 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
595
Forks
773
Avg merge
2d 6h
Merged PRs (30d)
18

Description

If I type `log download `, I want to see `all`, as well as the list of possible log file names to download.
Ranges should autocomplete as well as from.

Digging into this, it seems there a bug in `rline.py`.
The following code should allow completions, but does not.
```python3

class LogModule(mp_module.MPModule):
def __init__(self, mpstate):
super(LogModule, self).__init__(mpstate, "log", "log transfer")
self.add_command('log', self.cmd_log, "log file handling",
['',
'download all',
'download latest',
'download range (LOGNUM) (LOGNUM)',
'download from (LOGNUM)',
'download (LOGNUM) (LOGFILENAME)'])
self.add_completion_function('(LOGNUM)', self.complete_lognum)
self.add_completion_function('(LOGFILENAME)', self.complete_logfilename)
self.reset()

def complete_lognum(self, text):
'''complete log numbers known from a previous "log list"'''
return [str(lognum) for lognum in sorted(self.entries.keys())
if str(lognum).startswith(text)]

def complete_logfilename(self, text):
'''complete log filenames for logs known from a previous "log list"'''
return [self.default_log_filename(lognum) for lognum in sorted(self.entries.keys())
if self.default_log_filename(lognum).startswith(text)]
```

Claude suggested the following test:
```python3
'''
Regression test for a bug in MAVProxy.modules.lib.rline.complete_rule().

Bug summary
-----------
complete_rule() walks a rule's space-separated components to find the one
that should be expanded for the *current* argument position:

rule_components = rule.split(' ')
for i in range(len(cmd)-1):
if not rule_match(rule_components[i], cmd[i]):
return []
expanded = rule_expand(rule_components[len(cmd)-1], cmd[-1])

If every earlier token in `cmd` happens to match the rule's earlier
components (via rule_match(), which also expands "" alternations),
but the rule has *fewer* components than `cmd` has words, the final line
indexes `rule_components[len(cmd)-1]` past the end of the list and raises
an uncaught IndexError.

Because complete_rules() calls complete_rule() for every rule in a plain
loop with no exception handling, this single bad rule aborts completion
for the *entire* command -- every other, perfectly valid rule for that
command silently contributes nothing, and the user gets no completions
at all.

This is triggered in practice by a very natural rule list shape: a
single-token catch-all alternation for the first word of a command,
alongside longer rules that share one of those same alternatives as a
literal prefix, e.g.:

['',
'download all',
'download latest']

Typing "download a" should suggest "all", but instead crashes.
'''

import pytest

from MAVProxy.modules.lib import rline


class FakeMPState:
'''minimal stand-in for mpstate, just enough for rule_expand() to work'''
def __init__(self):
self.completion_functions = {}


def test_minimal_reproduction_indexerror():
'''smallest possible reproduction of the underlying rline.py bug'''
rline.rline_mpstate = FakeMPState()

rules = [
'', # 1-component rule; "foo" is also a literal prefix below
'foo baz', # 2-component rule: position 1 should offer "baz"
]

# user has typed "foo b" -- expect "baz" back, not a crash
cmd = ['foo', 'b']

with pytest.raises(IndexError):
# this SHOULD return ['baz']; instead the '' rule crashes
# complete_rules() before 'foo baz' ever gets a chance to run
rline.complete_rules(rules, cmd)


def test_log_download_reproduction():
'''
same bug, using the shape of rules that showed up while adding tab
completion to the "log" module's "download" sub-command.
'''
rline.rline_mpstate = FakeMPState()
rline.rline_mpstate.completion_functions['(LOGNUM)'] = lambda text: []
rline.rline_mpstate.completion_functions['(LOGFILENAME)'] = lambda text: []

rules = [
'',
'download all',
'download latest',
'download range (LOGNUM) (LOGNUM)',
'download from (LOGNUM)',
'download (LOGNUM) (LOGFILENAME)',
]

# user has typed "log download a" -- expect "all" to show up
cmd = ['download', 'a']

with pytest.raises(IndexError):
rline.complete_rules(rules, cmd)


if __name__ == '__main__':
import sys
sys.exit(pytest.main([__file__, '-v']))

```

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in rline.py with complete_rule() and complete_rules(), then run the supplied reproductions using the overlapping completion rules. Verify that an extra command word no longer aborts completion and that log download offers all, log filenames, and range/from argument completions.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
cli
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
70/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.