[ros2launch] legal tab completion of launch files #126

sloretz · 2020-02-21T01:41:01Z

Fixes #118

This fixes one issue, but it took multiple steps:

LaunchFileNameCompleter belongs on the launch_file_name argument, not launch_arguments.
Suppress completions of launch_arguments, since by default it completes all files in the current directory
~~Expose get_launch_file_paths so I can use it in a new completer~~ Add is_launch_file and expose it in API
Add a completer that allows first argument to be either a package name or a launch file

Here's the folder structure I used for testing

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

2 directories, 5 files

Here are the completions I tested with this PR

$ ros2 launch com<tab><tab>
common_interfaces       composition             composition/            composition_interfaces  composition_launch.py
$ ros2 launch composition<tab><tab>
composition             composition/            composition_interfaces  composition_launch.py
$ 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
$ ros2 launch composition c<tab>(v completes to v)
$ ros2 launch composition composition_demo.launch.py
$ ros2 launch composition/<tab><tab>
composition/asdf.launch.py  composition/foo/            composition/foo.launch.py
$ ros2 launch composition/foo<tab><tab>
composition/foo/           composition/foo.launch.py
$ ros2 launch composition/foo/<tab>(v completes to v)
$ ros2 launch composition/foo/ba
$ ros2 launch composition/foo/baz<tab>(vcompletes to v)
$ ros2 launch composition/foo/baz.launch.py

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

sloretz · 2020-02-21T01:42:40Z

CI (testing just ros2launch)

Linux
Linux-aarch64
macOS
Windows
Windows-container

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

sloretz · 2020-02-24T18:45:17Z

CI (testing just ros2launch)

Linux
Linux-aarch64
macOS
Windows
Windows-container

dirk-thomas · 2020-02-25T16:04:57Z

ros2launch/ros2launch/command/launch.py

        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()


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

kislyuk/argcomplete#289

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.

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

It does.

What version of argcomplete are you using?

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

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

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

sloretz · 2020-02-25T16:45:28Z

@dirk-thomas as of 71ae581 the PR now reuses FilesCompleter to do the path and directory completions

CI (testing just ros2launch)

Linux
Linux-aarch64
macOS
Windows
Windows-container

ros2launch/ros2launch/api/api.py

ros2launch/ros2launch/command/launch.py

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

dirk-thomas

Beside the comment about the function argument style this works for me.

ros2launch/ros2launch/api/api.py

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

sloretz · 2020-02-25T23:46:56Z

CI (testing just ros2launch)

Linux
Linux-aarch64
macOS
Windows
Windows-container

sloretz · 2020-02-27T18:26:34Z

Approved, and test failure appears unrelated (compiler warning in C code, but this PR only changes python code). Merging.

sloretz added 5 commits February 20, 2020 14:00

Complete launch file name for launch files not arguments

5a53491

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Expose get_launch_file_paths

29a6503

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Suppress invalid file completions

380bdc9

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Complete launch files in first argument

a656408

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Don't complete recursive launch files

8c04d21

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

sloretz added bug Something isn't working in review labels Feb 21, 2020

sloretz self-assigned this Feb 21, 2020

sloretz added 2 commits February 20, 2020 17:43

Typo in comment

b81028a

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

style

00f8903

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

dirk-thomas reviewed Feb 25, 2020

View reviewed changes

sloretz added 2 commits February 25, 2020 08:42

Add is_launch_file and expose it

753d630

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Reuse argcomplete machinery for paths

71ae581

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

sloretz requested a review from dirk-thomas February 25, 2020 16:43

dirk-thomas reviewed Feb 25, 2020

View reviewed changes

ros2launch/ros2launch/api/api.py Outdated Show resolved Hide resolved

ros2launch/ros2launch/api/api.py Outdated Show resolved Hide resolved

ros2launch/ros2launch/command/launch.py Outdated Show resolved Hide resolved

sloretz added 2 commits February 25, 2020 09:17

dict() instead of dictionary comprehension

31df98f

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

Remove unnecessary basename() call

96bd7f8

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

ros2 deleted a comment from sloretz Feb 25, 2020

Compatibility with argcomplete==1.8.1

84b564e

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

sloretz requested a review from dirk-thomas February 25, 2020 19:03

dirk-thomas approved these changes Feb 25, 2020

View reviewed changes

ros2launch/ros2launch/api/api.py Outdated Show resolved Hide resolved

is_launch_file takes normal arg

7b64ba9

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>

sloretz merged commit 5f54cb2 into master Feb 27, 2020

delete-merged-branch bot deleted the sloretz/ros2launch_legal_completion branch February 27, 2020 18:26

ahcorde mentioned this pull request May 8, 2020

Fix ros2 launch #147

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ros2launch] legal tab completion of launch files #126

[ros2launch] legal tab completion of launch files #126

sloretz commented Feb 21, 2020 •

edited

Loading

sloretz commented Feb 21, 2020

sloretz commented Feb 24, 2020

dirk-thomas Feb 25, 2020

sloretz Feb 25, 2020

sloretz Feb 25, 2020

dirk-thomas Feb 25, 2020

sloretz Feb 25, 2020

dirk-thomas Feb 25, 2020

sloretz commented Feb 25, 2020

dirk-thomas left a comment

sloretz commented Feb 25, 2020

sloretz commented Feb 27, 2020

[ros2launch] legal tab completion of launch files #126

[ros2launch] legal tab completion of launch files #126

Conversation

sloretz commented Feb 21, 2020 • edited Loading

sloretz commented Feb 21, 2020

sloretz commented Feb 24, 2020

dirk-thomas Feb 25, 2020

Choose a reason for hiding this comment

sloretz Feb 25, 2020

Choose a reason for hiding this comment

sloretz Feb 25, 2020

Choose a reason for hiding this comment

dirk-thomas Feb 25, 2020

Choose a reason for hiding this comment

sloretz Feb 25, 2020

Choose a reason for hiding this comment

dirk-thomas Feb 25, 2020

Choose a reason for hiding this comment

sloretz commented Feb 25, 2020

dirk-thomas left a comment

Choose a reason for hiding this comment

sloretz commented Feb 25, 2020

sloretz commented Feb 27, 2020

sloretz commented Feb 21, 2020 •

edited

Loading