eslint-plugin-unicorn icon indicating copy to clipboard operation
eslint-plugin-unicorn copied to clipboard

Enforce directory casing in `filename-case`

Open SimenB opened this issue 4 years ago • 16 comments

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

SimenB avatar Apr 16 '20 18:04 SimenB

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?

sarayourfriend avatar Dec 22 '21 14:12 sarayourfriend

I third this, it would be a fantastic addition to this already great library.

thislooksfun avatar Mar 03 '22 06:03 thislooksfun

Would love to see this supported as well.

risharma avatar Sep 09 '22 18:09 risharma

@sindresorhus What's the status of this? Can I write a PR?

moshest avatar Sep 16 '22 17:09 moshest

This issue is labeled as "help wanted". So yes, a good PR is welcome :)

sindresorhus avatar Sep 17 '22 13:09 sindresorhus

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.

mrmckeb avatar Nov 16 '22 05:11 mrmckeb

PR up! Any feedback would is welcome.

mrmckeb avatar Nov 16 '22 06:11 mrmckeb

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?

mrmckeb avatar Nov 17 '22 00:11 mrmckeb

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.

moshest avatar Nov 17 '22 09:11 moshest

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 😃

SimenB avatar Nov 18 '22 19:11 SimenB

Mostly every release of unicorn is major and breaking, who cares about it

dimaMachina avatar Nov 18 '22 19:11 dimaMachina

Exactly. Respect it's a breaking change, but don't let that stop us. (imo)

SimenB avatar Nov 18 '22 22:11 SimenB

Unfortunately that wasn't clear to me in the initial issue, @SimenB.

  • I agree that people should enforce consistent casing throughout, which is why I didn't introduce separate options for folder/directory casing. But not everyone feels that way (see @moshest's comment).
  • The rule is named 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.

mrmckeb avatar Nov 21 '22 02:11 mrmckeb

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

fisker avatar Nov 21 '22 03:11 fisker

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.

mrmckeb avatar Nov 28 '22 08:11 mrmckeb

If anyone wants to work on this, see the initial attempt and feedback in https://github.com/sindresorhus/eslint-plugin-unicorn/pull/1964

sindresorhus avatar Oct 28 '23 17:10 sindresorhus