Skip to content

Conversation

@disc7
Copy link
Contributor

@disc7 disc7 commented Mar 12, 2024

First pass.

Todo

  • unittest coverage
  • update Readme

@disc7 disc7 marked this pull request as draft March 12, 2024 21:13
.nyc_output
coverage
*.code-workspace
/src/config/config.local.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file looks ignored by mistake

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to include this file but in a way that Typescript is happy with if the values are commented out.

Copy link
Member

@TwitchBronBron TwitchBronBron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work on this.

We already support loading settings from a bslint.json file, so I don't think we should be adding another type of config (config.local). You should be able to define this new structure within existing bslint.json design. So I think the files in src/config/ should all be removed.

Here's how the config file is loaded via the CLI:

const config = normalizeConfig(options);

And here's how the config is loaded via the brighterscript plugins (and language server):
const { rules, fix, checkUsage, globals, ignores } = normalizeConfig(program.options);

I'd envision this being added as some subtype of the rules, maybe like this (with defaults set if the user didn't provide these). I'm open to suggestions on the exact structure. @elsassph any thoughts?

{
    "color-cert": {
        "severity": "error",
         "colors": {
             "broadcastSafe": {
                "black": "#161616",
                "white": "#EBEBEB"
            }
        }
    }
 }

@charlie-abbott-deltatre
Copy link
Contributor

Will take a look when I get some time. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants