-
Notifications
You must be signed in to change notification settings - Fork 43
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
nsort sorts namespaces and pages together #111
Conversation
When nsort sorting option is switched on, namespaces and pages on the same index level are sorted together. I use it with msort option as well. Original behaviour sorted namespaces first and then sorted pages were added. Maybe new sorting option can be added for this mode.
foreach($dirs_and_files as $dir_or_file) { | ||
$data[] = $dir_or_file; | ||
// MI: directory | ||
if(array_search($dir_or_file, $dirs_tmp, true) !== false && $dir_or_file['return']) { |
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.
Quick response:
Does the item `$dir_or_file not contain a property specific for folder vs file? Or is it otherwise not cheaper to add such a property?
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 have not studied the code enough to answer, because PHP is not my prefferred language ;-)
These changes are what suits me in my case now...
Please feel free to make any modifications with better knowledge of the background.
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.
The item array has the entry: $dir_or_file['type']
.
The value of this entry may be equal to d
(unfolded d irectory), l
(directory, l oad content of directory later) or f
(f ile). So you can check for $dir_or_file['type'] != 'f'
.
Hmm, I don't know how I can add commits to your fork. I think I will look how I can add some tests. I like to ensure that the way of handling empty folders is maintained.
Thanks for this great addition! I think this is worth an sorting option. Not every one needs always sorting the dirs and the files together. This is useful for the backward compatibility as well. I expect that the performance will not derogate too much, but did you test this maybe with some larger data sets already? |
I have not tested the performance much. Anyway some property like IsDirectory would help on items instead of searching directories array. |
Is there any chance that this PR gets merged in the official master? |
This would sure be a nice improvement for those working with "book" and "chapter" formats. |
I still need to test this, for some different situations. If others have time for testing, please be welcome to do and report what you test and whether the results were like your expectations. |
FWIW, I applied this to my installation and it works like a charm, thank you! What I did:
|
Which version is your installation? |
Thanks for the advice. I would like to add, that the options nsort AND tsort have to be switched on for this to work (tested with the current release 'Hogfather'). |
Similar to samuelet#111 but updated for IndexMenu 5.0 release.
@rolfik Thanks for contributing this long time ago and your patience! I have implemented it manually due to conflicts with the changes by @eduardomozart due to conflicts. |
When nsort sorting option is switched on, namespaces and pages on the same index level are sorted together.
I use it with msort option as well.
Original behaviour sorted namespaces first and then sorted pages were added.
Maybe new sorting option can be added for this mode.