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

Implement album sorting #192

Merged
merged 3 commits into from Jan 18, 2016
Merged

Implement album sorting #192

merged 3 commits into from Jan 18, 2016

Conversation

@trapperhoney
Copy link
Contributor

@trapperhoney trapperhoney commented Jan 18, 2016

This adds an 'albums_sort_attr' configuration modeled off of the
'medias_sort_attr'. It defaults to the existing behavior of sorting
sub-albums by directory name. It allows sort by any metadata attribute
stored in the album metadata file.

The existing album sort test wasn't really verifying album sorting
behavior, so I just re-wrote the whole thing to include tests for the
old behavior and new.

This should resolve issue #121

This adds an 'albums_sort_attr' configuration modeled off of the
'medias_sort_attr'.  It defaults to the existing behavior of sorting
sub-albums by directory name.  It allows sort by any metadata attribute
stored in the album metadata file.

The existing album sort test wasn't really verifying album sorting
behavior, so I just re-wrote the whole thing to include tests for the
old behavior and new.

This should resolve issue #121
@saimn
Copy link
Owner

@saimn saimn commented Jan 18, 2016

Looks good, thanks @trapperhoney !
Could you remove the .index.md.swp file and add yourself to the AUTHORS file ?

@trapperhoney
Copy link
Contributor Author

@trapperhoney trapperhoney commented Jan 18, 2016

Woops, sorry I missed the tmp file :\ I'd rather not include my name in the distributed source. Is the github trail not good enough?

@saimn
Copy link
Owner

@saimn saimn commented Jan 18, 2016

It can be your github pseudonym if you prefer, no problem with this.

saimn added a commit that referenced this pull request Jan 18, 2016
@saimn saimn merged commit ea3eae4 into saimn:master Jan 18, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@saimn
Copy link
Owner

@saimn saimn commented Jan 18, 2016

Thanks !

@trapperhoney
Copy link
Contributor Author

@trapperhoney trapperhoney commented Jan 18, 2016

Thank you!

@saimn saimn added this to the 1.1.0 milestone Feb 15, 2016
@saimn saimn mentioned this pull request Feb 15, 2016
@tohojo
Copy link
Contributor

@tohojo tohojo commented Apr 26, 2016

How is this supposed to work? I tried adding an 'Order' field to index.md of each of my subdirectories and setting that as album_sort_attr. However, I just get an AttributeError when trying to build the gallery.

I ended up adding this to _get_metadata() of the Album class, but that's obviously not a workable solution to have to do that for all the interesting fields:

    try:
        self.order = self.meta['order'][0]
    except KeyError:
        self.order = self.settings.get('order')
@saimn
Copy link
Owner

@saimn saimn commented Apr 26, 2016

@tohojo - Indeed, you're right, it works only with attributes of the Album objects ... Copying all the meta items as object attributes seems not a good option, so maybe we could allow some special syntax for albums_sort_attr, something like 'albums_sort_attr': ('meta', 'Order') ?

@tohojo
Copy link
Contributor

@tohojo tohojo commented Apr 26, 2016

Ah, good, so it wasn't just me who was missing something! :)

Yeah, having a way to specify meta fields would be good (thought that was what it was meant for). Maybe just falling back to looking in the metadata object if the attribute is not found on the Album object itself? That would avoid introducing new syntax... Not sure if there are field names that would clash in surprising ways, though?

@saimn
Copy link
Owner

@saimn saimn commented Apr 26, 2016

Maybe just falling back to looking in the metadata object if the attribute is not found on the Album object itself? That would avoid introducing new syntax..

Yep, that's another possibility. Maybe both could be allowed to resolve potential conflicts.

@tohojo
Copy link
Contributor

@tohojo tohojo commented Apr 26, 2016

@trapperhoney
Copy link
Contributor Author

@trapperhoney trapperhoney commented Apr 26, 2016

For what it's worth, the same (lack of metadata support) semantics apply to the sorting behavior of individual items and the items_sort_attr configuration option.

@saimn
Copy link
Owner

@saimn saimn commented Apr 26, 2016

@tohojo - Great ! No string preference, meta.fieldname can work too.
@trapperhoney - Yes, good point. The media objects have more useful attributes, but their metadata is not accessible for the sort.

@tohojo
Copy link
Contributor

@tohojo tohojo commented Apr 27, 2016

Created #202 :)

kontza pushed a commit to kontza/sigal that referenced this pull request Aug 28, 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
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants