style: data_types by umarcor · Pull Request #482 · VUnit/vunit

This PR contains some unrelated python changes.

This is because of:

This PR is based on #488 and it should be merged later.

There is a single python change besides those, and it is pertinent: https://github.com/VUnit/vunit/pull/482/files/aa21c26acbb529aa71dcdfef822486a02a8ceb26..738942aa00150c3a83b079061dfd9551a9345581

Also the coding style looks weird compared to the other part of the code base. Typically we use a single line for short functions and line break after the function name for long functions.

I have seen that, and that's why I decided to modify almost all the sources in subdir data_types, and not only the ones that I will modify in upcoming PRs (integer_*, string_*).

The main motivation in favour of the 'new style' is to have 'fixed' locations to make it easier to find subprogram names, arguments/params, the type of the return value, and the type of the params.

Keeping the type of the subprogram in the same line as the name does not allow it, because names of different subprograms are not aligned vertically. Moreover, params after the first one can have multiple possible locations and the vertical aligment is different for each of them. This does not only depend on the number of params, but on the actual names and the length of the type names. The same applies to the return type, which can be located in three different lines, and the vertical aligment is different.

The most disturbing characteristic of the current style for me is that it is hard to know how many params do one-liners have. You need to read the line carefully. This is specially so because : and ; are very similar. For example, it is not easy to spot that these two subprograms are 'complementary':

procedure set(dict : dict_t; key : string := "default"; value : string) is
impure function get(dict : dict_t ; key : string := "default") return string is
procedure
set(
  dict  : dict_t;
  key   : string := "default"
  value : string
);

impure function
get(
  dict : dict_t;
  key  : string := "default"
) return string;

I'm ok with using the following alternative style:

procedure set (
  dict  : dict_t;
  key   : string := "default"
  value : string
);

impure function get (
  dict : dict_t;
  key  : string := "default"
) return string;

Overall, I'd like you to assess the constraint of putting each param in a different line, considering the return type or closing parenthesis as an additional param.

Have a look at vhdl/com/src/com_api.vhd to get a feel for the coding style. We dont have any code that breaks right after the function/procedure keyword.

As commented, this is so that subprogram names are located at a fixed position (indentation) which does not depend on the type of subprogram. In some contexts, procedures and functions can have the same name and arguments: i.e. the same subprogram that returns an exit code or does not. Nonetheless, disregarding this change, the modifications that the 'new style' might imply in vhdl/com/src/com_api.vhd are:

  • Move the trailing ); of the procedures to a new line.
  • Move the trailing ) return *; of the functions to a new line.
  • Convert one-liners to the multi-line format.