Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ros2launch] legal tab completion of launch files #126

Merged
merged 13 commits into from
Feb 27, 2020
Merged

[ros2launch] legal tab completion of launch files #126

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
5a53491
Complete launch file name for launch files not arguments
sloretz Feb 20, 2020
29a6503
Expose get_launch_file_paths
sloretz Feb 21, 2020
380bdc9
Suppress invalid file completions
sloretz Feb 21, 2020
a656408
Complete launch files in first argument
sloretz Feb 21, 2020
8c04d21
Don't complete recursive launch files
sloretz Feb 21, 2020
b81028a
Typo in comment
sloretz Feb 21, 2020
00f8903
style
sloretz Feb 24, 2020
753d630
Add is_launch_file and expose it
sloretz Feb 25, 2020
71ae581
Reuse argcomplete machinery for paths
sloretz Feb 25, 2020
31df98f
dict() instead of dictionary comprehension
sloretz Feb 25, 2020
96bd7f8
Remove unnecessary basename() call
sloretz Feb 25, 2020
84b564e
Compatibility with argcomplete==1.8.1
sloretz Feb 25, 2020
7b64ba9
is_launch_file takes normal arg
sloretz Feb 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions ros2launch/ros2launch/api/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from launch.launch_description_sources import InvalidLaunchFileError
from launch.launch_description_sources import InvalidPythonLaunchFileError

from .api import get_launch_file_paths
from .api import get_share_file_path_from_package
from .api import launch_a_launch_file
from .api import LaunchFileNameCompleter
Expand All @@ -27,6 +28,7 @@


__all__ = [
'get_launch_file_paths',
'get_share_file_path_from_package',
'InvalidLaunchFileError',
'InvalidPythonLaunchFileError',
Expand Down
53 changes: 51 additions & 2 deletions ros2launch/ros2launch/command/launch.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@

from ament_index_python.packages import get_package_prefix
from ament_index_python.packages import PackageNotFoundError
from argcomplete.completers import SuppressCompleter
from ros2cli.command import CommandExtension
from ros2launch.api import get_launch_file_paths
from ros2launch.api import get_share_file_path_from_package
from ros2launch.api import launch_a_launch_file
from ros2launch.api import LaunchFileNameCompleter
Expand All @@ -26,6 +28,52 @@
from ros2pkg.api import package_name_completer


class SuppressCompleterWorkaround(SuppressCompleter):
"""Workaround https://github.com/kislyuk/argcomplete/pull/289 ."""

def __call__(self, *args, **kwargs):
"""Make SupressCompleter callable by returning no completions."""
return ()


def package_name_or_launch_file_completer(prefix, parsed_args, **kwargs):
# Complete package names
completions = list(package_name_completer(prefix=prefix, **kwargs))

# list of 2-tuples: (directory, part of prefix to prepend)
dirs_to_check = []

if os.path.isdir(prefix) and not prefix.endswith(os.sep):
# if prefix is directory 'foo' then suggest 'foo/'
completions.append(prefix + os.sep)
if os.path.isdir(prefix) and prefix.endswith(os.sep):
# if prefix is 'foo/' then check 'foo/' for launch files
dirs_to_check.append((prefix, prefix))
if not prefix.endswith(os.sep) and os.path.isdir(os.path.dirname(prefix)):
# if prefix is 'foo/bar' and then check 'foo' for launch files
dirname = os.path.dirname(prefix)
basename = os.path.basename(prefix)
prepend = prefix[:-len(basename)]
dirs_to_check.append((dirname, prepend))
if os.sep not in prefix:
# if prefix is 'foo' then check current directory for files starting with 'foo'
dirs_to_check.append((os.curdir, ''))

for dirname, prepend in dirs_to_check:
# complete launch files in a directory
for launch_file in get_launch_file_paths(path=dirname):
if os.path.normpath(os.path.dirname(launch_file)) == os.path.normpath(dirname):
# get_launch_file_paths() is recursive; complete only launch files in first level
completions.append(prepend + os.path.basename(launch_file))

# complete directories since they may contain launch files
for path in os.listdir(path=dirname):
if os.path.isdir(os.path.join(dirname, path)):
completions.append(prepend + path + os.sep)

return completions


class LaunchCommand(CommandExtension):
"""Run a launch file."""

Expand All @@ -49,17 +97,18 @@ def add_arguments(self, parser, cli_name):
arg = parser.add_argument(
'package_name',
help='Name of the ROS package which contains the launch file')
arg.completer = package_name_completer
arg.completer = package_name_or_launch_file_completer
arg = parser.add_argument(
'launch_file_name',
# TODO(wjwwood) make this not optional when full launch path is supported.
nargs='?',
help='Name of the launch file')
arg.completer = LaunchFileNameCompleter()
arg = parser.add_argument(
'launch_arguments',
nargs='*',
help="Arguments to the launch file; '<name>:=<value>' (for duplicates, last one wins)")
arg.completer = LaunchFileNameCompleter()
arg.completer = SuppressCompleterWorkaround()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is SuppressCompleter not sufficient? It seems to work in other places, e.g. https://github.com/colcon/colcon-package-information/blob/8360ca881e0c51edd8696f3683029badcc301835/colcon_package_information/verb/list.py#L95-L100

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kislyuk/argcomplete#289

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate why it does work without the workaround in other cases but not here?

Using

$ tree
.
├── composition
│   ├── asdf.launch.py
│   ├── foo
│   │   ├── bar.launch.py
│   │   └── baz.launch.py
│   └── foo.launch.py
├── composition_launch.py
└── file_i_dont_want_completed.txt

With SupressCompleter()

$ ros2 launch composition <tab><tab>
composition/                    composition_launch.py           file_i_dont_want_completed.txt

With SupressCompleterWorkaround()

$ ros2 launch composition <tab><tab>
-a                              --debug                         --print-description             --show-args
composition_demo.launch.py      -p                              -s                              --show-arguments
-d                              --print                         --show-all-subprocesses-output

I haven't tested colcon-package-information, so I don't know if SuppressCompleter is working as intended there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if SuppressCompleter is working as intended there.

It does.

What version of argcomplete are you using?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

argcomplete==1.11.1, and I was also able to reproduce using upstream master

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am missing the part why it is necessary in this case only.


def main(self, *, parser, args):
"""Entry point for CLI program."""
Expand Down