Unity 8

Merge lp:~nick-dedekind/unity8/StrFTimeFormatter into lp:unity8

StrFTimeFormatter
Merge into trunk

Proposed by Nick Dedekind on 2013-10-23

Status:	Merged
Approved by:	Michał Sawicz on 2013-12-18
Approved revision:	473
Merged at revision:	598
Proposed branch:	lp:~nick-dedekind/unity8/StrFTimeFormatter
Merge into:	lp:unity8
Prerequisite:	lp:~larsu/unity8/lp1236413
Diff against target:	239 lines (+67/-11) 11 files modified plugins/Unity/Indicators/CMakeLists.txt (+0/-1) plugins/Unity/Indicators/Messaging/qml/SimpleTextMessage.qml (+2/-1) plugins/Unity/Indicators/Messaging/qml/SnapDecision.qml (+2/-2) plugins/Unity/Indicators/plugin.cpp (+0/-2) plugins/Utils/CMakeLists.txt (+1/-0) plugins/Utils/plugin.cpp (+3/-0) plugins/Utils/timeformatter.cpp (+30/-1) plugins/Utils/timeformatter.h (+15/-3) tests/plugins/Unity/Indicators/CMakeLists.txt (+0/-1) tests/plugins/Utils/CMakeLists.txt (+1/-0) tests/plugins/Utils/timeformattertest.cpp (+13/-0)
To merge this branch:	bzr merge lp:~nick-dedekind/unity8/StrFTimeFormatter
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
PS Jenkins bot (community)	continuous-integration		Approve on 2013-12-18
Michał Sawicz			Approve on 2013-12-18
Lars Karlitski (community)		2013-10-23	Approve on 2013-10-24
Review via email: mp+192343@code.launchpad.net

Commit message

Added parser for strftime in TimeFormatter.
Moved TimeFormatter to Utils plugin.

Description of the change

Added parser for strftime in TimeFormatter.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-23:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1480/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Lars Karlitski (larsu) wrote on 2013-10-23:

This patch doesn't check the return value of strftime() and might thus pass an uninitialized buffer into QString().

According to strftime(3), the return value of that function is 0 when the resulting string is longer than the passed in buffer. However, it can also be 0 when the resulting string is legitimately empty. For example, when the format string is "".

I think the best solution is to prepend a " " to the format string. That way, the return value is never smaller than 1, unless there's an error. And then simply return QString(buffer + 1).

You could also use g_date_time_format(), which supports all of the POSIX formats as well.

review: Needs Fixing

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2013-10-24:

> This patch doesn't check the return value of strftime() and might thus pass an
> uninitialized buffer into QString().
>
> According to strftime(3), the return value of that function is 0 when the
> resulting string is longer than the passed in buffer. However, it can also be
> 0 when the resulting string is legitimately empty. For example, when the
> format string is "".
>
> I think the best solution is to prepend a " " to the format string. That way,
> the return value is never smaller than 1, unless there's an error. And then
> simply return QString(buffer + 1).
>
> You could also use g_date_time_format(), which supports all of the POSIX
> formats as well.

GDateTime it is.
Renamed to GDateTimeFormatter as well.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-10-24:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1487/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Lars Karlitski (larsu) wrote on 2013-10-24:

Great, thanks.

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-07:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1594/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-11-07:

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1600/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-11-21:

What did we decide in the end on that? Should this come pre-formatted from the indicator?

review: Needs Information

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2013-11-28:

> What did we decide in the end on that? Should this come pre-formatted from the
> indicator?

No, we would need to subscribe to tz changes and update each event in the indicator service and update the actions. The indicator should keep processing to a minimum (times are read in as unix time).

Revision history for this message

Nick Dedekind (nick-dedekind) wrote on 2013-11-28:

> What did we decide in the end on that? Should this come pre-formatted from the
> indicator?

I think this is still the correct way to do it. I'm not sure where the indicator gets it's format string from, but perhaps it should be a gsettting somewhere.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-12-10:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1851/rebuild

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-12-11:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1864/rebuild

review: Approve (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-12-13:

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/1897/rebuild

review: Approve (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2013-12-18:

Ohkay ;P

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2013-12-18:

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) on 2013-12-18:

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Mohammad Rakibul Hasan

Nick Dedekind

Unity Team

1	=== modified file 'plugins/Unity/Indicators/CMakeLists.txt'
2	--- plugins/Unity/Indicators/CMakeLists.txt 2013-10-16 19:48:57 +0000
3	+++ plugins/Unity/Indicators/CMakeLists.txt 2013-12-13 13:24:49 +0000
4	@@ -36,7 +36,6 @@
5	unitymenumodelcache.cpp
6	unitymenumodelstack.cpp
7	visibleindicatorsmodel.cpp
8	- timeformatter.cpp
9	)
10	add_definitions(-DUNITYINDICATORS_LIBRARY)
11
12
13	=== modified file 'plugins/Unity/Indicators/Messaging/qml/SimpleTextMessage.qml'
14	--- plugins/Unity/Indicators/Messaging/qml/SimpleTextMessage.qml 2013-10-16 19:48:57 +0000
15	+++ plugins/Unity/Indicators/Messaging/qml/SimpleTextMessage.qml 2013-12-13 13:24:49 +0000
16	@@ -21,6 +21,7 @@
17	import QtQuick 2.0
18	import Ubuntu.Components 0.1
19	import Unity.Indicators 0.1
20	+import Utils 0.1 as Utils
21
22	HeroMessage {
23	id: __heroMessage
24	@@ -50,7 +51,7 @@
25	opacity: 0.0
26	enabled: false
27
28	- TimeFormatter {
29	+ Utils.TimeFormatter {
30	id: timeFormatter
31	time: __heroMessage.time
32	format: "hh:mm - MMM dd"
33
34	=== modified file 'plugins/Unity/Indicators/Messaging/qml/SnapDecision.qml'
35	--- plugins/Unity/Indicators/Messaging/qml/SnapDecision.qml 2013-10-16 19:48:57 +0000
36	+++ plugins/Unity/Indicators/Messaging/qml/SnapDecision.qml 2013-12-13 13:24:49 +0000
37	@@ -20,8 +20,8 @@
38
39	import QtQuick 2.0
40	import Ubuntu.Components 0.1
41	-import Unity.Indicators 0.1 as Indicators
42	import Unity.Indicators 0.1
43	+import Utils 0.1 as Utils
44
45	HeroMessage {
46	id: snapDecision
47	@@ -59,7 +59,7 @@
48	height: units.gu(4)
49	opacity: 0.0
50
51	- TimeFormatter {
52	+ Utils.TimeFormatter {
53	id: timeFormatter
54	time: snapDecision.time
55	format: "hh:mm - MMM dd"
56
57	=== modified file 'plugins/Unity/Indicators/plugin.cpp'
58	--- plugins/Unity/Indicators/plugin.cpp 2013-10-16 19:48:57 +0000
59	+++ plugins/Unity/Indicators/plugin.cpp 2013-12-13 13:24:49 +0000
60	@@ -33,7 +33,6 @@
61	#include "unitymenumodelcache.h"
62	#include "unitymenumodelstack.h"
63	#include "visibleindicatorsmodel.h"
64	-#include "timeformatter.h"
65
66	static QObject* menuModelCacheSingleton(QQmlEngine* engine, QJSEngine* scriptEngine) {
67	Q_UNUSED(engine);
68	@@ -52,7 +51,6 @@
69	qmlRegisterType<RootActionState>(uri, 0, 1, "RootActionState");
70	qmlRegisterType<ModelPrinter>(uri, 0, 1, "ModelPrinter");
71	qmlRegisterType<VisibleIndicatorsModel>(uri, 0, 1, "VisibleIndicatorsModel");
72	- qmlRegisterType<TimeFormatter>(uri, 0, 1, "TimeFormatter");
73
74	qmlRegisterSingletonType<UnityMenuModelCache>(uri, 0, 1, "UnityMenuModelCache", menuModelCacheSingleton);
75
76
77	=== modified file 'plugins/Utils/CMakeLists.txt'
78	--- plugins/Utils/CMakeLists.txt 2013-11-22 17:11:03 +0000
79	+++ plugins/Utils/CMakeLists.txt 2013-12-13 13:24:49 +0000
80	@@ -16,6 +16,7 @@
81	mediaartcache.cpp
82	qlimitproxymodelqml.cpp
83	qsortfilterproxymodelqml.cpp
84	+ timeformatter.cpp
85	unitymenumodelpaths.cpp
86	plugin.cpp
87	)
88
89	=== modified file 'plugins/Utils/plugin.cpp'
90	--- plugins/Utils/plugin.cpp 2013-11-22 17:11:03 +0000
91	+++ plugins/Utils/plugin.cpp 2013-12-13 13:24:49 +0000
92	@@ -30,6 +30,7 @@
93	#include "bottombarvisibilitycommunicatorshell.h"
94	#include "qlimitproxymodelqml.h"
95	#include "qsortfilterproxymodelqml.h"
96	+#include "timeformatter.h"
97	#include "unitymenumodelpaths.h"
98
99	static const char* BOTTOM_BAR_VISIBILITY_COMMUNICATOR_DBUS_PATH = "/BottomBarVisibilityCommunicator";
100	@@ -42,6 +43,8 @@
101	qmlRegisterType<QLimitProxyModelQML>(uri, 0, 1, "LimitProxyModel");
102	qmlRegisterType<QSortFilterProxyModelQML>(uri, 0, 1, "SortFilterProxyModel");
103	qmlRegisterType<UnityMenuModelPaths>(uri, 0, 1, "UnityMenuModelPaths");
104	+ qmlRegisterType<TimeFormatter>(uri, 0, 1, "TimeFormatter");
105	+ qmlRegisterType<GDateTimeFormatter>(uri, 0, 1, "GDateTimeFormatter");
106	qmlRegisterUncreatableType<BottomBarVisibilityCommunicatorShell>(uri, 0, 1, "BottomBarVisibilityCommunicatorShell", "Can't create BottomBarVisibilityCommunicatorShell");
107	}
108
109
110	=== renamed file 'plugins/Unity/Indicators/timeformatter.cpp' => 'plugins/Utils/timeformatter.cpp'
111	--- plugins/Unity/Indicators/timeformatter.cpp 2013-10-16 19:48:57 +0000
112	+++ plugins/Utils/timeformatter.cpp 2013-12-13 13:24:49 +0000
113	@@ -167,6 +167,35 @@
114
115	void TimeFormatter::update()
116	{
117	- priv->timeString = QDateTime::fromMSecsSinceEpoch(priv->time / 1000).toString(priv->format);
118	+ priv->timeString = formatTime();
119	Q_EMIT timeStringChanged(priv->timeString);
120	}
121	+
122	+QString TimeFormatter::formatTime() const
123	+{
124	+ return QDateTime::fromMSecsSinceEpoch(time() / 1000).toString(format());
125	+}
126	+
127	+GDateTimeFormatter::GDateTimeFormatter(QObject* parent)
128	+: TimeFormatter(parent)
129	+{
130	+}
131	+
132	+QString GDateTimeFormatter::formatTime() const
133	+{
134	+ gchar* time_string;
135	+ GDateTime* datetime;
136	+ QByteArray formatBytes = format().toUtf8();
137	+
138	+ datetime = g_date_time_new_from_unix_local(time());
139	+ if (!datetime) {
140	+ return "";
141	+ }
142	+
143	+ time_string = g_date_time_format(datetime, formatBytes.constData());
144	+ QString formattedTime(QString::fromUtf8(time_string));
145	+
146	+ g_free(time_string);
147	+ g_date_time_unref(datetime);
148	+ return formattedTime;
149	+}
150
151	=== renamed file 'plugins/Unity/Indicators/timeformatter.h' => 'plugins/Utils/timeformatter.h'
152	--- plugins/Unity/Indicators/timeformatter.h 2013-10-16 19:48:57 +0000
153	+++ plugins/Utils/timeformatter.h 2013-12-13 13:24:49 +0000
154	@@ -19,10 +19,10 @@
155	#ifndef TIME_FORMATTER_H
156	#define TIME_FORMATTER_H
157
158	-#include "unityindicatorsglobal.h"
159	#include <QObject>
160
161	-class UNITYINDICATORS_EXPORT TimeFormatter : public QObject
162	+// TODO - bug #1260728
163	+class TimeFormatter : public QObject
164	{
165	Q_OBJECT
166	Q_PROPERTY(QString format READ format WRITE setFormat NOTIFY formatChanged)
167	@@ -31,7 +31,7 @@
168
169	public:
170	TimeFormatter(QObject *parent = 0);
171	- ~TimeFormatter();
172	+ virtual ~TimeFormatter();
173
174	QString format() const;
175	QString timeString() const;
176	@@ -47,8 +47,20 @@
177	void timeStringChanged(const QString &timeString);
178	void timeChanged(qint64 time);
179
180	+protected:
181	+ virtual QString formatTime() const;
182	+
183	private:
184	struct TimeFormatterPrivate *priv;
185	};
186
187	+class GDateTimeFormatter : public TimeFormatter
188	+{
189	+public:
190	+ GDateTimeFormatter(QObject *parent = 0);
191	+
192	+protected:
193	+ virtual QString formatTime() const;
194	+};
195	+
196	#endif
197
198	=== modified file 'tests/plugins/Unity/Indicators/CMakeLists.txt'
199	--- tests/plugins/Unity/Indicators/CMakeLists.txt 2013-11-06 10:26:58 +0000
200	+++ tests/plugins/Unity/Indicators/CMakeLists.txt 2013-12-13 13:24:49 +0000
201	@@ -40,4 +40,3 @@
202	indicator_test(menucontentactivatortest ADDITIONAL_CPPS ${INDICATORS_DIR}/menucontentactivator.cpp)
203	indicator_test(unitymenumodelstacktest ADDITIONAL_CPPS ${TEST_DIR}/mocks/QMenuModel/unitymenumodel.cpp ${INDICATORS_DIR}/unitymenumodelstack.cpp)
204	indicator_test(rootactionstatetest ADDITIONAL_LIBS IndicatorsQml)
205	-indicator_test(timeformattertest ADDITIONAL_LIBS IndicatorsQml)
206
207	=== modified file 'tests/plugins/Utils/CMakeLists.txt'
208	--- tests/plugins/Utils/CMakeLists.txt 2013-06-05 22:03:08 +0000
209	+++ tests/plugins/Utils/CMakeLists.txt 2013-12-13 13:24:49 +0000
210	@@ -24,4 +24,5 @@
211	run_tests(
212	qlimitproxymodeltest
213	qsortfilterproxymodeltest
214	+ timeformattertest
215	)
216
217	=== renamed file 'tests/plugins/Unity/Indicators/timeformattertest.cpp' => 'tests/plugins/Utils/timeformattertest.cpp'
218	--- tests/plugins/Unity/Indicators/timeformattertest.cpp 2013-10-16 20:17:15 +0000
219	+++ tests/plugins/Utils/timeformattertest.cpp 2013-12-13 13:24:49 +0000
220	@@ -48,6 +48,19 @@
221
222	QCOMPARE(formatter.timeString(), time.toString(format));
223	}
224	+
225	+ void testFormatStrF()
226	+ {
227	+ const QString format = "%d-%m-%Y %I:%M%p";
228	+
229	+ QDateTime time = QDateTime::currentDateTime();
230	+
231	+ GDateTimeFormatter formatter;
232	+ formatter.setTime(time.toMSecsSinceEpoch() / 1000); // strftime in seconds since epoc
233	+ formatter.setFormat(format);
234	+
235	+ QCOMPARE(formatter.timeString(), time.toString("dd-MM-yyyy hh:mmAP"));
236	+ }
237	};
238
239	QTEST_GUILESS_MAIN(TimeFormatterTest)