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 support for custom DateColumn timestamp formats #2366

Merged
merged 5 commits into from Sep 22, 2017

Conversation

Projects
None yet
3 participants
@Meriipu
Contributor

Meriipu commented Apr 25, 2017

Personally I dislike the random "Yesterday", "23:55:12", "30/01/2015", and so on timestamps scattered through the datecolumn

This adds an option in advanced preferences to specify the format string to be used.

@Meriipu Meriipu force-pushed the Meriipu:dateformat_dev branch 2 times, most recently from b866f88 to 3b9d3de Apr 25, 2017

@Meriipu Meriipu closed this Apr 25, 2017

@Meriipu Meriipu force-pushed the Meriipu:dateformat_dev branch from 3b9d3de to 0f30e75 Apr 25, 2017

@Meriipu Meriipu reopened this Apr 25, 2017

@Meriipu

This comment has been minimized.

Contributor

Meriipu commented Apr 25, 2017

I can not say I know what the failed check is for but ok

@Meriipu

This comment has been minimized.

Contributor

Meriipu commented Apr 25, 2017

weird

@lazka

This comment has been minimized.

Member

lazka commented May 6, 2017

What is the format string or behavior that you prefer?

Regarding the failed check, this is an old gtk+ bug which sometimes leads to crashes. I've opened #2371

@Meriipu

This comment has been minimized.

Contributor

Meriipu commented May 6, 2017

I prefer having a date (year, month, day) and optionally a timestamp

@Meriipu

This comment has been minimized.

Contributor

Meriipu commented Jun 16, 2017

trying to add in the new changes to the file from head to the branch being merged seems to have done more harm than not

@Meriipu Meriipu force-pushed the Meriipu:dateformat_dev branch from 1e44264 to 0834a88 Jun 28, 2017

@Meriipu

This comment has been minimized.

Contributor

Meriipu commented Jun 28, 2017

hopefully it rebased now.

but yeah this is just something I did to solve one of my gripes with gtk3 (applications? it is hard to not put too much of the blame for anything on gtk itself at times) tending to use a bit too mixed/unprecise labels for timestamps for my liking.

@Meriipu Meriipu force-pushed the Meriipu:dateformat_dev branch from e9e31f3 to 0834a88 Jun 28, 2017

@declension

This comment has been minimized.

Member

declension commented Jun 28, 2017

Looks reasonable to me, though would prefer if this had some tests around the now-uncovered code, i.e. testing what happens when the new config entry is set.

"datecolumn_timestamp_format")
# use default behaviour-format
if not format_setting:

This comment has been minimized.

@declension

declension Jun 28, 2017

Member

Why not swap the order of the if-else and avoid the not (always harder to read)?

This comment has been minimized.

@Meriipu

Meriipu Jun 28, 2017

Contributor

I suppose it could be swapped. I started with == "" and changed it to not s in an attempt to fix some failed check (mentioned earlier), and at that point the new order maybe makes more sense.

I can try to look into how tests work to add some.

This comment has been minimized.

@Meriipu

Meriipu Jun 28, 2017

Contributor

Would something like setting it to a format string and running test_qltk_songlistcolumns.py again work? I have no idea how one would test it, other than maybe setting the current time to a specific value and checking whether a date-column formats to what was expected with some entry.

@declension

@Meriipu you could test it there yes.

Injecting a date to a test song, configuring the new date preferences, then checking the output is what you expect sounds easy enough.

format_ = "%A"
else:
format_ = "%x"
# use format from Advanced Preferences

This comment has been minimized.

@declension

declension Jun 29, 2017

Member

Well, it's not really from the advanced preferences.

@Meriipu

This comment has been minimized.

Contributor

Meriipu commented Jun 29, 2017

Should I include a test for the standard behaviour (Say making "~#added" a date that was yesterday and comparing with "Yesterday"), or is testing for the new format enough (this standard behaviour might change after all sometime in the future, maybe)?

@Meriipu Meriipu force-pushed the Meriipu:dateformat_dev branch from 26647e0 to 68e08e1 Jun 29, 2017

stamp = int(time.mktime(d.timetuple()))
column = create_songlist_column("~#added")
text = self._render_column(column, **{"~#added": stamp})
self.assertEqual(text, "19990501 23:11:59 PLAINTEXT")

This comment has been minimized.

@declension

declension Jun 30, 2017

Member

Good stuff 👍

def test_nonconfigured_datecol_format(self):
text = quodlibet.config.gettext("settings",
"datecolumn_timestamp_format")
self.assertEqual(text, "")

This comment has been minimized.

@declension

declension Jun 30, 2017

Member

Perhaps this test (or a separate) should do a quick check to see that the default behaviour hasn't been changed. Bit harder as you said because of the way the existing one works but you could set the date as above and at the very least check that it isn't the above...

"datecolumn_timestamp_format")
# use format configured in Advanced Preferences
if format_setting:

This comment has been minimized.

@declension
@Meriipu

This comment has been minimized.

Contributor

Meriipu commented Jun 30, 2017

Maybe it is possible to set it to more than 7 days ago, and under the assumptions tests run under the US locale, expect it to be formatted as "%x", though I am not sure about that and it seems unreliable.

@declension declension merged commit cf0659d into quodlibet:master Sep 22, 2017

7 checks passed

ci/circleci: job.debian8.py2 Your tests passed on CircleCI!
Details
ci/circleci: job.debian8.py3 Your tests passed on CircleCI!
Details
ci/circleci: job.ubuntu16.04.py2 Your tests passed on CircleCI!
Details
ci/circleci: job.ubuntu16.04.py3 Your tests passed on CircleCI!
Details
ci/circleci: job.win32.py2 Your tests passed on CircleCI!
Details
ci/circleci: job.win32.py3 Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@declension

This comment has been minimized.

Member

declension commented Sep 22, 2017

Sorry about the delay. Thanks @Meriipu

@Meriipu Meriipu deleted the Meriipu:dateformat_dev branch Sep 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment