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

Making plural a bit smarter #22

Closed
wants to merge 2 commits into from

Conversation

richmilns
Copy link

No description provided.

@richmilns
Copy link
Author

I've fixed the CI warnings so everything passes now. Hope it's useful! Thanks for your work on the classes, they are really nice to work with.

@jjgrainger
Copy link
Owner

Hi @richmilns

Thanks for the pull requests and fixing the errors, all looks great.

The only concern I have is how far it goes? By keeping it stupid (adding an 's' at the end) leaves it to the developer to set the plural name.

Once you start adding more logic to account for other plurals, it's no longer stupid but also not as smart as it could be! Where this could forever be worked on in an attempt to accommodate for all plural possibilities!

That being said we could always set up a method to handle the logic and maybe use arrays to map the different plurals?

Again, thanks for the pull, I'll see about adding this in alongside other work that needs to be done.

Cheers :)

@richmilns
Copy link
Author

No problem at all!

I do agree with what you're saying because what I have add in this PR does not cover all possible scenarios, however what it does do is automatically handle English words like 'Categories' (which originally came out as 'Categorys') automatically. I think this is a nice improvement and a bit of a time saver.

My thinking was that if the majority of scenarios were covered then that might be enough, and the other minority cases can be addressed manually by the developer of the theme or plugin easily using the API you already provide.

Is there a roadmap for the project by the way? I'm adding some other features to my fork, but I don't want to reinvent the wheel :)

@jjgrainger
Copy link
Owner

After looking at this again, I believe the best way forward is to create something similar when moving to traits for certain methods.

Closing this out in favour of #40

Thanks!

@jjgrainger jjgrainger closed this Apr 12, 2020
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.

None yet

2 participants