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.

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

addons/namingng.py: Reinstate dict-with-regexps, cosmetic overhaul. #5824

Merged
merged 2 commits into from
Jan 4, 2024

Commits on Jan 4, 2024

  1. addons/namingng.py: Reinstate dict-with-regexps feature.

    In 7c168fa (Feature/ros naming check, Jan 9,
    2019) a feature was implemented where regular expressions can be given in a
    dict, i.e. `{"[a-z]+":[arg0,arg1]}` instead of an array `["[a-z]+"]`.
    
    There is only the namingng.py code, an example `ROS_naming.json` and some
    GitHub discussion to go at when trying to understand the feature. The feature
    was not readily understandable by looking only at the code. It tested the first
    argument using `if ~arg0` (i.e. using bit-wise negation), which only evaluates
    as true when `arg0 == -1`, while the example JSON and discussion indicate that
    `arg0` is actually boolean. The code seemed to indicate some form where `arg1`
    is not used, and `arg0` would contain a string, but it was not obvious how this
    should work.
    
    This feature seems to be intended as follows: When specifying a list, a
    positive match of each item is required. When specifying a dict, the dict keys
    are regular expressions and their values are `[bool,string]` with bool
    indicating what match result is to be reported as an error, and string a
    message that is appended to the reported error. This approach has various
    benefits: the config can specify an explanatory message, and it is now possible
    to more simply forbid and/or require certain patterns.
    
    Recent changes, especially 98b9244
    (addons/namingng.py: Add tests for include guards, config file validation,
    Jan 2 2024) completely broke this feature, while keeping the code in. This
    patch provides a rewrite of the feature, adds sanity checks to the
    configuration, and provides unit tests for both the configuration of the
    feature and the feature itself.
    
    The dict feature does not work for `RE_FILE` (yet). The example config file
    `ROS_naming.json` is modified to prevent an error triggered by the dict value
    of `RE_FILE`.
    mvds00 committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    416eeb5 View commit details
    Browse the repository at this point in the history
  2. addons/namingng.py: Cosmetic overhaul, reduce indents and function le…

    …ngth.
    
    This patch is (/should be) purely cosmetic, and aims to improve maintainability
    by reducing indentation, function length and redundant code/comments.
    
    The following was done:
    - put every check in a separate function with the same signature
    - use args.debugprint instead of function kwarg debugprint
    - eliminate almost-duplicate code
    - simplify/remove redundant code, e.g. `if a is None or a != 'Public'`
    - remove stale or all-too-obvious comments
    - remove unused arguments
    - reorder code flow to reduce indentation (`if not a: continue`)
    - fix recent, minor bug that prevented standalone mode to work properly
    
    The results:
    - the number of "tabs" is reduced by 30%
    - the maximum indentation depth is now 5, coming from 8
    - the longest function is now ~75 lines, coming from ~150 lines
    - 9 functions were added
    - a net total of 6 lines of code were removed
    mvds00 committed Jan 4, 2024
    Configuration menu
    Copy the full SHA
    f4ec7e9 View commit details
    Browse the repository at this point in the history