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

Added method Utils::add_irregular for managing irregular names #265

Merged
merged 4 commits into from
Apr 18, 2014
Merged

Added method Utils::add_irregular for managing irregular names #265

merged 4 commits into from
Apr 18, 2014

Conversation

bashaus
Copy link
Contributor

@bashaus bashaus commented Mar 8, 2013

This pull request adds a new method to the Utils class which allows for the dynamic addition of irregular nouns. This method is for those special cases.

Use case:
We have a table name called "Skus" which would be singularised to "Skus" because the ending of the word was "us". We added the "add_irregular" method to circumvent this issue.

@jpfuentes2
Copy link
Owner

Thanks a lot for the patch! Would you mind fixing your whitespace so that it follows our project's convention?

@al-the-x
Copy link
Collaborator

Thanks for the contribution, @bashaus.

IMO, automatic inflection of table names just adds a big hunk of complexity that isn't necessary. Is making a dev match the classname to the table name or provide the name of his table in each class definition that onerous? Not trying to start a flame war... So many other ORMs use this logic, maybe there's room for a separate library to perform this functionality at this point...?

@al-the-x al-the-x closed this Apr 11, 2013
@al-the-x
Copy link
Collaborator

Sorry, didn't mean to close this. O_O

@al-the-x al-the-x reopened this Apr 11, 2013
@bashaus
Copy link
Contributor Author

bashaus commented Apr 12, 2013

While I agree that it would be a great idea to have an external inflection library controlling names, I think this solution is a quick fix for those people who encountered the same problem that we did (described above).

@al-the-x
Copy link
Collaborator

@bashaus Absolutely agree. Don't mind my kvetching... ;)

@bluefuton
Copy link

I've encountered the same issue with the inflector and this is an effective workaround. It'd be great to see this pulled in.

@floodedcodeboy
Copy link

Agreed, this would be a great workaround. +1

@al-the-x al-the-x mentioned this pull request Jul 29, 2013
@bashaus
Copy link
Contributor Author

bashaus commented Apr 10, 2014

Hey @jpfuentes2 , this one has been floating around for a while. Can we get this merged for the next release?

@jpfuentes2
Copy link
Owner

Can you merge master into your branch? There's a single conflict:

diff --cc lib/Utils.php
index 3f18baa,bdb37e6..0000000
--- a/lib/Utils.php
+++ b/lib/Utils.php
@@@ -355,10 -362,4 +362,14 @@@ class Util
        {
                return preg_replace("/$char+/",$char,$string);
        }
 -}
++<<<<<<< HEAD
 +
 +      public static function add_irregular($singular, $plural)
 +      {
 +              self::$irregular[$singular] = $plural;
 +      }
 +};
 +?>
++=======
++}
++>>>>>>> master

@koenpunt
Copy link
Collaborator

I suggest rebasing.

But, I also vote for an external library for inflections, as a configurable option of course. For example my php-inflector which I have been using in production for years now, but is only available as package since today. (earlier version was is my php-rails endeavor)

@jpfuentes2
Copy link
Owner

Definitely rebase :)

@jpfuentes2
Copy link
Owner

@bashaus , if you can take care of that I will merge.

Conflicts:
	lib/Utils.php
@bashaus
Copy link
Contributor Author

bashaus commented Apr 18, 2014

@jpfuentes2 - ready when you are, thanks!

jpfuentes2 added a commit that referenced this pull request Apr 18, 2014
Added method Utils::add_irregular for managing irregular names
@jpfuentes2 jpfuentes2 merged commit 0f04186 into jpfuentes2:master Apr 18, 2014
@koenpunt koenpunt mentioned this pull request Jul 17, 2014
27 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants