`indentStyle` and `indentSize` fields of rule config are camelCase, which is inconsistent with other rule config fields.

[JHertz5]

Environment

$ ./bin/vsg -v
VHDL Style Guide (VSG) version: 3.20.0.dev8
Git commit SHA: 23b63ebf

Describe the bug Issue #1095 came about because the user expected the indentStyle and indentSize fields to follow the snake_case convention that the vast majority of rule config fields follow, i.e. they expected the fields to be called indent_style and indent_size.

To Reproduce Steps to reproduce the behavior:

1. Run ./bin/vsg -rc constant_400 (or any other rule)
2. Note names

   ...
     "indentStyle": "spaces",
     "indentSize": 2,
   ...

Expected behavior I would expect these fields to follow the snake_case convention of other rules' fields.

[JHertz5]

@jeremiah-c-leary, I'm happy to raise a PR for this if you're ok with the change.

[jeremiah-c-leary]

I have been pondering this for a bit. It looks like indentStyle and indentSize are the only options that are camelCase.

I believe the way forward on this is to change the options to snake case, but also accept the camelCase and throw a warning that it will eventually be deprecated.

I will work on a solution.

--Jeremy

[jeremiah-c-leary]

Afternoon @JHertz5 ,

I finally pushed my update for this to the issue-1096 branch.

I added a warning if indentSize or indentType is found in any configuration, while also converting them to indent_size and indent_type under the hood.

Let me know what you think.

Regards,

--Jeremy

[JHertz5]

Hi @jeremiah-c-leary. Thanks very much for this! I've given your branch a try and I'm able to trigger the warning that you mentioned, but I'm not sure that the under the hood conversion is working.

---

With this config:

rule:
  global:
    indent_style: 'smart_tabs'
    indent_size: 3
    # indentStyle: 'smart_tabs'
    # indentSize: 3

I get the following -rc output

$ ./bin/vsg -c ./test.yml -rc constant_001
{
  "rule": {
    "constant_001": {
      "indent_style": "smart_tabs",
      "indent_size": 3,
      "phase": 4,
      "disable": false,
      "fixable": true,
      "severity": "Error"
    }
  }
}

(note that the values of indent_style and indent_size differ from the default) and I see --fix changing files to tab indentation.

---

However, with this config:

rule:
  global:
    # indent_style: 'smart_tabs'
    # indent_size: 3
    indentStyle: 'smart_tabs'
    indentSize: 3

I get

$ ./bin/vsg -c ./test.yml -rc constant_001
Warning in configuration file ./test.yml: option indentSize will be deprecated in a future release, change to indent_size.
{
  "rule": {
    "constant_001": {
      "indent_style": "spaces",
      "indent_size": 2,
      "phase": 4,
      "disable": false,
      "fixable": true,
      "severity": "Error"
    }
  }
}

(note that the wrning is triggered, but indent_style and indent_size are set to their defaults) and I see --fix changing the previously tabbed file back to space indentation.

Perhaps the deprecated keys are detected and removed from the config in config.py before they get to rule.py where they would be converted?

I also noticed that in the function lBlah in config.py, there is a typo "lDeprectedKeyson line 127 (should belDeprecatedKeys`) which might cause problems.

[jeremiah-c-leary]

Morning @JHertz5 ,

Part of me is thinking changing indentSize and indentStyle to their new values under the hood is the wrong approach. When a rule is deprecated, the user is notified so they can update their configuration. In this case an option has been deprecated, so why do I feel the code must handle this under the hood? I think I should just throw an error and force the user to update. This would keep the code cleaner as the code attempting to convert indentSize and indentStyle would be removed.

Would you have a preference?

--Jeremy

[JHertz5]

Hi @jeremiah-c-leary! I agree, it would definitely be preferable to make a clean transition IMO. The only benefit to the under-the-hood conversion would be that it gives people time to switch over before their configs are broken, but even then, you're just breaking that config later instead of now. If I'm not mistaken, trying to use a deprecated rule throws an error, so it makes sense that using a deprecated config option would do the same.

[jeremiah-c-leary]

Evening @JHertz5 ,

I changed the code to raise an exception if indentSize or indentType is found. The message states to change them to indent_size and indent_type respectively. Could you check it out and let me know what you think of the implementation?

Thanks,

--Jeremy

[JHertz5]

Hi @jeremiah-c-leary. That looks mostly good to me, but I've found a mistake. indentStyle has been renamed to indent_style, but in config.py and in some tests, the code is using indentType as the deprecated name and indent_type as the new name. As a result, using indentStyle doesn't raise the aforementioned error and is silently ignored instead.

[jeremiah-c-leary]

Morning @JHertz5 ,

Good catch. Not sure how I changed indent_style to indent_type.

BTW, it turns out the smart_tab tests were not being executed because the directory was missing __init__.py. I made some changes to include those tests. I discovered this when trying to grep for indentStyle and wondering why the test was not failing.

I pushed an update. Let me know if there are any other issues.

Thanks,

--Jeremy

[JHertz5]

Hi @jeremiah-c-leary. Excellent, thanks very much! I've tested the branch again and it is looking good to me!