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

Hash::sort()'s $dir accepts SORT_DIR/SORT_ASC ? #13392

Closed
o0h opened this issue Jul 3, 2019 · 3 comments

Comments

@o0h
Copy link
Contributor

commented Jul 3, 2019

This is a (multiple allowed):

  • bug

  • enhancement

  • feature-discussion (RFC)

  • CakePHP Version: CakePHP 2/3

What you did

I feel Hash::sort() is similar to Collection::sortBy().
But $dir param is '(str)asc|desc' in Hash method, against Collection method accept (int)SORT_ASC|SORT_DESC.
These difference are a bit confusioning.

>>> \Cake\Utility\Hash::sort($array, '{n}.class', SORT_DESC)
=> [
     [
       "class" => "c",
     ],
     [
       "class" => "b",
     ],
     [
       "class" => "a",
     ],
   ]
>>> collection($array)->sortBy('class', SORT_DESC)->toList()
=> [
     [
       "class" => "a",
     ],
     [
       "class" => "b",
     ],
     [
       "class" => "c",
     ],
   ]

What happened

SORT_ASC wth Hash::sort is not working.

What you expected to happen

Hash::sort() accepts SORT_xxx constants.

Many developers use IDE or editors which has code complete feature, so working with constants will benefit for their DX.

@ravage84 ravage84 added this to the 3.8.1 milestone Jul 3, 2019

@markstory

This comment has been minimized.

Copy link
Member

commented Jul 4, 2019

Seems like a reasonable idea to me.

@o0h

This comment has been minimized.

Copy link
Contributor Author

commented Jul 4, 2019

Thanks!
Well then, I try to solve this and send pull request.

o0h added a commit to o0h/cakephp that referenced this issue Jul 5, 2019

markstory added a commit that referenced this issue Jul 7, 2019

@markstory markstory modified the milestones: 3.8.1, 3.8.2 Jul 13, 2019

@markstory

This comment has been minimized.

Copy link
Member

commented Jul 13, 2019

Closing as a pull request was merged into 3.next

@markstory markstory closed this Jul 13, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.