Open SimenB opened 4 years ago
I'm happy to help with the implementation of this. Over on WordPress/Openverse we're running into a related issue of wanting to be able to ignore based on the full path of the file, not just the filename itself.
For example, given the following directory structure:
components
└── VComponent
├── meta
│ └── utility.js
├── README.md
├── VComponent.vue
└── VComponentPart.vue
We'd like to be able to ignore everything in the meta
folder. Currently that's not possible because of the use of path.basename
in the rule to extract the filename from the file path.
Regardless of whether this proposal for enforcing directory casing in this rule, it would be helpful to be able to match ignores against the folder path somehow.
Some considerations that come to mind, particularly around backwards compatability with existing rule configurations and ignores, is that an existing ignore pattern could accidentally exclude a folder that was previously included in the rule. To accomodate that, folder matching could be its own option for this rule that would default to false
?
I third this, it would be a fantastic addition to this already great library.
Would love to see this supported as well.
@sindresorhus What's the status of this? Can I write a PR?
This issue is labeled as "help wanted". So yes, a good PR is welcome :)
I don't see a PR open for this, so just pinging to say that I'm going to try to get this one in.
PR up! Any feedback would is welcome.
Hi all, I wanted to get some feedback on the PR for this issue (#1964). The way it works right now is that it's opt-in, via includePath
(boolean
), which I believe solves the core issue here (CC @SimenB).
@fisker suggested introducing this as a breaking change so that it's enabled by default, and also that we introduce additional configuration to allow different conventions for directories than files. I believe this is a much bigger change, and I don't have capacity to deliver it at the moment.
I was hoping to get some insights into what the participants in this thread want (or need) from this feature. Is the current implementation enough to solve your needs? Or do you need advanced configuration?
My opinion is that this feature SHOULD support different settings for folders and files. Many projects prefer to have CamelCase for folders but Kebab for files.
I think the best way to implement this and also prevent breaking changes is to introduce another role foldername-case
.
IMO it should enforce the entire path (within cwd) regardless of files and folders. No option - you either care about consistent casing or not. Yes, a breaking change, but that's just a number 😃
Mostly every release of unicorn is major and breaking, who cares about it
Exactly. Respect it's a breaking change, but don't let that stop us. (imo)
Unfortunately that wasn't clear to me in the initial issue, @SimenB.
filename-case
, which could be confusing to people if it starts checking the entire path without an opt-in/out. I'm not worried about a breaking change as much as breaking behaviour unexpectedly.We could ship the rule today as minor change with an opt-in - as it's ready to go - and wait for further feedback before pushing forward in another direction.
Stop thinking about BREAKING/NON-BRAKING, just make it correct, it's never been something we care. https://github.com/sindresorhus/eslint-plugin-unicorn/issues/686#issuecomment-1320449464
I believe the current PR meets the criteria in the outlined in the description of this issue. It also has test coverage for the new opt-in behaviour.
However, if people feel that the additional features are a blocker, I might be able to circle-back to this in the next few weeks, but it will probably slip until after Christmas as the suggested changes would require a moderate amount of refactoring, along with updating around 170 test cases.
If anyone wants to work on this, see the initial attempt and feedback in https://github.com/sindresorhus/eslint-plugin-unicorn/pull/1964
@fisker @sindresorhus, I have some time to come back to this and was about to start work when I saw #2186 - which I think impacts this a little (CC @michaelowolf).
The feedback from @fisker in the PR was that we should default to checking the path, but then we also had feedback that people wanted to have different settings for directories than files. @fisker proposed the following rule settings:
{
directories: {
cases: "kebabCase",
ignore: ["..."]
},
filename: {
cases: {
"camelCase": true,
"pascalCase": true
},
ignore: ["..."]
},
To ensure that the needs of #2186 are still met, I propose the following:
type Case = "camelCase" | "pascalCase"; // Incomplete
type Settings = {
/**
* Restrict to one or more cases globally.
* Note that configuration for path parts will override this value.
**/
restrictTo: Case | Case[];
/** Ignore for any global or path part configuration. **/
ignore: Glob[];
/** Set rules that only apply to directories. **/
directories: {
restrictTo: Case | Case[];
ignore: Glob[];
};
/** Set rules that only apply to filenames. **/
filename: {
restrictTo: Case | Case[];
ignore: Glob[];
};
/** Set rules that only apply additional filename extensions, i.e. `file.test.ts`. **/
additionalExtensions: {
restrictTo: Case | Case[];
ignore: Glob[];
};
};
For many people, I suspect the config itself will end up being very small:
{
restrictTo: "kebabCase",
ignore: ["..."]
},
Not in scope:
./kebab-case/PascalCase/kebab-case.ts
).Once everyone has had a chance to provide feedback, I can take a look at this.
We'd like to enforce casing of both files and directories. Is that possible, or would it need some kind of "root" directory to avoid caring about directory name outside of the project? Easy enough to achieve with js config files I guess. There is
context.getCwd()
, could probably work as well