Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enforce directory casing in filename-case #686

Open
SimenB opened this issue Apr 16, 2020 · 16 comments
Open

Enforce directory casing in filename-case #686

SimenB opened this issue Apr 16, 2020 · 16 comments

Comments

@SimenB
Copy link

SimenB commented Apr 16, 2020

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

@sarayourfriend
Copy link

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?

@thislooksfun
Copy link

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

@risharma
Copy link

risharma commented Sep 9, 2022

Would love to see this supported as well.

@moshest
Copy link

moshest commented Sep 16, 2022

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

@sindresorhus
Copy link
Owner

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

@mrmckeb
Copy link

mrmckeb commented Nov 16, 2022

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
Copy link

mrmckeb commented Nov 16, 2022

PR up! Any feedback would is welcome.

@mrmckeb
Copy link

mrmckeb commented Nov 17, 2022

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?

@moshest
Copy link

moshest commented Nov 17, 2022

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.

@SimenB
Copy link
Author

SimenB commented Nov 18, 2022

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 😃

@dimaMachina
Copy link
Contributor

dimaMachina commented Nov 18, 2022

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

@SimenB
Copy link
Author

SimenB commented Nov 18, 2022

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

@mrmckeb
Copy link

mrmckeb commented Nov 21, 2022

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.

@fisker
Copy link
Collaborator

fisker commented Nov 21, 2022

Stop thinking about BREAKING/NON-BRAKING, just make it correct, it's never been something we care. #686 (comment)

@mrmckeb
Copy link

mrmckeb commented Nov 28, 2022

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.

@sindresorhus
Copy link
Owner

If anyone wants to work on this, see the initial attempt and feedback in #1964

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

Successfully merging a pull request may close this issue.

9 participants