opened
gep13
username_0
To allow for suppressing individual rules.

Relates to #121
[WIP] Implement ConfigurationHandler
issue
created
gep13
username_0
@username_2 @username_3 would either of you be able to have a review of this PR?  Thanks!
comment
created
gep13
username_0
This allows for configuration similar to the following:

```
{
  "chocoLanguageServer":{
    "language": {
      "suppressedRules": [
        "CHOCO0002"
      ]
    }
  }
}
```

**NOTE:** None of this is fixed in stone, just to show how things can be done.
comment
created
AdmiringWorm
username_1
So far it looks good to me
comment
created
gep13
username_0
@username_1 @username_4 @username_2 I have done a fair bit of refactoring here, but I think it is all for the greater good.

- Refactored so that only one rule per validation
- This allowed adding an Id and DocumentationUrl property to each rule
- Added ability to suppress a rule.  If it is suppressed, the Validate method will never be called on that rule

What are your collective thoughts on these changes?
comment
created
tintoy
username_2
Looks good to me - getting everything from the DI container will keep things simpler :)
comment
created
gep13
username_0
I am starting to play with this:

![image](https://user-images.githubusercontent.com/1271146/53688760-c7280200-3d40-11e9-9bfd-508940439e5b.png)

Trying to update the configuration, to add suppressions for a particular rule.  Things don't seem to be working though.  Anyone played with this?
comment
created
mholo65
username_3
@username_0 do you have some code for the above?
comment
created
gep13
username_0
@username_3 I have it locally, but I was struggling to get things working the way I want it to. I am thinking about splitting that work into a separate PR.
comment
created
mkevenaar
username_4
Have you looked how https://github.com/DavidAnson/vscode-markdownlint did it? It has a MIT license
comment
created
gep13
username_0
Also I think the CHOCO0### should not be ignorable, as they are a requirement.

I have had the same thought.  While it is correct to say that this is a requirement for pushing to Chocolatey.org, it may not be a requirement when pushing packages internally.  As such, I think I am going to leave the ability to suppress a requirement, but also put in another configuration parameter named something like `allowSuppressionOfRequirements` which will control whether this can be done or not.
comment
created
AdmiringWorm
username_1
👍
comment
created
gep13
username_0
@username_1 @username_4 I have implemented the suggestions that were left in this PR, and I have opened up some follow up issues here:

https://github.com/username_0/chocolatey-vscode/issues/140
https://github.com/username_0/chocolatey-vscode/issues/139

I am going to go ahead and merge this in, as it impacts on a lot of different sections, and I don't want to hold up any further work.  I have tested most of the changes, and it seems to be working fine.  If there are any issues found, we can correct them as we go.

As I mention in the related issue, this will be a breaking change, as I had to refactor the default configuration that is contributed by the extension.

Let me know if you have any questions about anything.
comment
created
gep13
username_0
@all-contributors please add @username_2 for ideas, review
comment