search

mayah / tinytoml

A header only C++11 library for parsing TOML
BSD 2-Clause "Simplified" License
167 stars 31 forks source link

Time type suggestions #10

Closed RobertBColton closed 8 years ago

RobertBColton commented 8 years ago

In the Qt Framework the DateTime, Date, and Time objects main conversion type is std::time_t. This conversion requires a rather long call:

return QVariant(QDateTime::fromTime_t(std::chrono::system_clock::to_time_t(value.as<std::time_t>())));

Is it possible that toml::Value could also provide .asstd::time_t() for time types? Is it possible that toml::Value could also distinguish between Date, Time, and DateTime? It would be useful in our Qt application which has separate QDateEdit, QTimeEdit, and QDateTime edit so as to not have to create the third control when either Date or Time is not needed.

mayah commented 8 years ago

On Linux, time_t is signed long, so I don't think it's good to have as<std::time_t>(), which is the same as as<signed long>(). It looks converting to an integer type.

It would be good to have as_time_t(), though.

mayah commented 8 years ago

Added as_time_t().

For naming convention, asTime_t(), asTimeT() etc. can be considered. However, I want to keep the case for time_t, so I chose as_time_t(). If you have an objection, feel free to make a pull request.

RobertBColton commented 8 years ago

I've no objections to that solution at all. Also I would like to say thank you for writing tinytoml, it is an excellently toml library and much better than the other implementations we tried. It does basically everything we need it to without much overhead or any headaches.

The only other request I have there is if I could tell the difference between a Date, a Time, and a DateTime value. The reason being that we generate our UI from the toml file. Where Qt's QVariant has DateType, TimeType, and DateTimeType there is only toml::Value::Time which does not distinguish in tinytoml. This is useful because then I know whether to create a QDateEdit, QTimeEdit, or QDateTimeEdit in Qt.

mayah commented 8 years ago

According to https://github.com/toml-lang/toml#datetime, toml does not distinguish time, date or datetime. We need to extend toml specification to distinguish them. Also, toml specification does not have a value like time (it has date and datetime, but it does not have time)?

So, currently I don't have any plan to add them by myself. The workaround would be to add type field or something in your toml file?

RobertBColton commented 8 years ago

Thank you, I see now it would actually be impossible with std::time_t because it is since the epoch. We're going to be using schema's down the road for the components anyway, so that solution will be fine.

I decided to go ahead and mention this to the toml guys just in case they may not have considered such a use case. https://github.com/toml-lang/toml/issues/412