Skip to content

chore: add kataw config, linter options and custom rules#173

Open
KFlash wants to merge 2 commits into
mainfrom
config
Open

chore: add kataw config, linter options and custom rules#173
KFlash wants to merge 2 commits into
mainfrom
config

Conversation

@KFlash
Copy link
Copy Markdown
Contributor

@KFlash KFlash commented Aug 2, 2021

This PR deal with a 'kataw.json' in the root folder.

It fetch the json file and process the data.

It then load lint rules - including custom rules - in the same way as ESLint.

It also fetch the 'minify' and 'prettify' options.

In the end it return a config file aka ESLint.

@KFlash
Copy link
Copy Markdown
Contributor Author

KFlash commented Aug 2, 2021

@aladdin-add Can you finish this PR? It's 99% complete, but I'm not 100% confident with ESLint internals.
At least you got your custom rules now.

Need to optimize and tweak the configuration

Linter rules default folder

/scr/linter/rules

Default config rules

kataw:recommended is located here:

/scr/configuration/config

@KFlash KFlash requested a review from aladdin-add August 2, 2021 12:24
@kataw kataw deleted a comment from lgtm-com Bot Aug 2, 2021
@aladdin-add
Copy link
Copy Markdown
Collaborator

aladdin-add commented Aug 3, 2021

I was writting a cli for kataw in JS(~50%, not pushed yet). but this one seems has a better completeness. :-( 

will look later. :)

@aladdin-add
Copy link
Copy Markdown
Collaborator

the main reason to use JS - dogfooding. :)

@KFlash
Copy link
Copy Markdown
Contributor Author

KFlash commented Aug 3, 2021

It's not a CLI, but loading config file and linter rules. The performance is horrible. It take near 157 ms to load while a simple parser load take 9 ms.
So you need to improve performance and get rid of all try / catch I put in there

@KFlash
Copy link
Copy Markdown
Contributor Author

KFlash commented Aug 3, 2021

Ideal if this could be re-written to use some kind of parallel processing and lazy loading, but I haven't used many of the NodejS I/O methods for years so I'm outdated :)

@kataw kataw deleted a comment from lgtm-com Bot Aug 4, 2021
@KFlash
Copy link
Copy Markdown
Contributor Author

KFlash commented Aug 8, 2021

@aladdin-add friendly ping

@aladdin-add
Copy link
Copy Markdown
Collaborator

seems the code has a few i/o related code; it will block users run it on non-node.js env (e.g. browsers).

what do you think put it in another package(build on the top of kataw core), so we can focus the public API in this one?

@KFlash
Copy link
Copy Markdown
Contributor Author

KFlash commented Aug 11, 2021

The code is to get the config file. Will it not be an issue if we put it in another package to get the linter rules that the compiler needs?

What kind of i/o blockers? It only load a config file, if linting is enabled it grab the rules and return.

This is only for CLI, and that is NodeJS. Not browsers

const config = getconfigFromCli() // the code in this PR

compiler(files, conig);

@KFlash
Copy link
Copy Markdown
Contributor Author

KFlash commented Aug 11, 2021

I'm less active now do to RL and I'm also experimenting with incremental parsing that will be part of the CLI

@aladdin-add
Copy link
Copy Markdown
Collaborator

aladdin-add commented Aug 11, 2021

well, another option is to add an entry "bin" to load the config. But let's do not do io related op in "src/index.ts".

it is the same as eslint doing: https://github.com/eslint/eslint/blob/3b6cd8934b3640ffb6fa49b471babf07f0ad769a/package.json#L6-L9

@KFlash
Copy link
Copy Markdown
Contributor Author

KFlash commented Aug 11, 2021

Ok. Can you remove from "index.ts" and move to a more propriate place and load from binary as in ESLint?

@KFlash
Copy link
Copy Markdown
Contributor Author

KFlash commented Aug 14, 2021

@aladdin-add any progress on this? I managed to get incremental parsing working. Need to add an CLI option for it.

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.

2 participants