-
Notifications
You must be signed in to change notification settings - Fork 103
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Showing
3 changed files
with
6 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8dc5d55
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey..good to see this library being used by alot of developers. I have used it on several API's.
I have a concern with this commit. I will use an example to raise my issue.
Lets say I want to fetch 5 records from 0, two times. Assuming its a paginated app. How will I pass the limits in your new limit method??
To my understanding, the number is the starting point while the offset is how many records I want.
This was my idea - from my pull request
but with your commit
Probably you can make it clear if I am confusing myself.
Thanks.
8dc5d55
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand, but Slim-PDO is made as query builder for the SQL syntax. According to the MySQL documentation for example,
offset
(if given) comes first:[LIMIT {[offset,] row_count | row_count OFFSET offset}]
So that's why I changed it. 😅
8dc5d55
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FaaPz can the parameter also support string?
IMO, it is a PITA if i need to casting every time I call limit:
Since I get the offset and limit from query param (used in pagination).
In my experience, other library is allow numeric string in limit parameters.
So, I need to read the source before I realized the parameter need to be integer.
I think using is_numeric in limit is more appropriate: