Allow correct languages on uncrustify

[MiguelCompany]

When running uncrustify -h it does not show C++ as a valid option for -l.

This PR changes the valid options on --language to C and CPP

Signed-off-by: Miguel Company MiguelCompany@eprosima.com

[mm318]

Is ament_uncrustify not translating C++ to CPP when passing it to uncrustify?

[mm318]

I don't think the C++ here is meant for uncrustify. I think it's meant for ament_uncrustify, and making this change will break the dictionary here.

Can you double check?

[audrow]

@mm318, can you confirm the link to the dictionary?

[audrow]

Nevermind.

[audrow]

After spending some time going through the code and trying uncrustify, I don't believe that this change breaks any dictionary.

@MiguelCompany I also think that the following line should also be changed:

ament_lint/ament_uncrustify/ament_uncrustify/main.py

Line 108 in 2d0d0d1

args.paths, {'C': c_extensions, 'C++': cpp_extensions},

Following the code

The language is only used here:

ament_lint/ament_uncrustify/ament_uncrustify/main.py

Lines 107 to 109 in 2d0d0d1

	files_by_language = get_files(
	args.paths, {'C': c_extensions, 'C++': cpp_extensions},
	excludes=args.exclude, language=args.language)

get_files creates a dictionary where the language is the value. It optionally uses the supplied language.

ament_lint/ament_uncrustify/ament_uncrustify/main.py

Lines 213 to 217 in 2d0d0d1

	extensions_with_dot_to_language = {
	f'.{extension}': language or ext_language
	for ext_language, extensions in extension_types.items()
	for extension in extensions
	}

This is ultimately used in grouping the files by language (file type is key, a list of files is the value):

ament_lint/ament_uncrustify/ament_uncrustify/main.py

Lines 236 to 239 in 2d0d0d1

	language = extensions_with_dot_to_language.get(ext, None)
	if language is not None:
	files[language].append(
	os.path.normpath(os.path.join(dirpath, filename)))

The uncrustify is invoked on each group of languages:

ament_lint/ament_uncrustify/ament_uncrustify/main.py

Lines 128 to 129 in 2d0d0d1

	output_files = invoke_uncrustify(
	uncrustify_bin, input_files, args, language, temp_path, suffix)

The actual command used to call uncrustify then appends all of the files as the last argument:

ament_lint/ament_uncrustify/ament_uncrustify/main.py

Lines 269 to 276 in 2d0d0d1

	cmd = [uncrustify_bin, '-c', args.config_file]
	if language:
	cmd += ['-l', language]
	cmd += [
	'--prefix', temp_path,
	'--suffix', suffix]
	cmd.extend(input_files)
	subprocess.check_output(cmd, cwd=cwd, stderr=subprocess.STDOUT)

[audrow]

Thanks for the PR @MiguelCompany. I think this is a good fix.

I think that the change should also be added in one other place in the file. See #272 (comment).

[mm318]

So following the code, the value of args.language will be passed to uncrustify?

[audrow]

the value of args.language will be passed to uncrustify

Correct, as the language (-l), if it is supplied. If not, the file extensions will be used. If I understand correctly, C++ isn't a valid language specifier for uncrustify (CPP is), but it still uses the supplied config file, so uncrustify will seem to handle the C++ correctly regardless of it ignoring the language specifier.

[MiguelCompany]

Thanks for the PR @MiguelCompany. I think this is a good fix.

I think that the change should also be added in one other place in the file. See #272 (comment).

I rebased, fixed DCO and also updated the dictionary key from C++ to CPP

[audrow]

• Linux
• Linux-aarch64
• macOS
• Windows

[audrow]

@mm318, does it look good to you, too?

[mm318]

Yep, looks good to me!

[audrow]

@MiguelCompany, thanks for the PR!