Patch : editing "Posted" time of transactions.

[John Ralls]

> On Jan 28, 2016, at 9:43 AM, gLETTERyYuMEANSj LETTERyOt <gletteryyumeansjletteryot at gmail.com> wrote:
> 
> 2015-05-28 17:15 UTC+02:00, John Ralls <jralls at ceridwen.us>:
>> Please don't submit patches to the mailing list. Open a bug in
>> Bugzilla.gnome.org or fork our repo on Github, create a branch with your
>> patch, and make a pull request. Patches should be written against HEAD in
>> the branch you're working on -- which should be master for a feature request,
>> especially one that changes the database or the way it's interpreted.
> 
> I have finally found some spare time to submit a patch this way. It is on
> 
> https://github.com/gijut/gnucash
> 
> It compiles without more warning than current version on x86_64, I have tested
> it for one hour.
> 
> It does no more prints any debugging line (so that the patch is easier to
> read).

BTW your https://github.com/gijut/gnucash/commit/885314788855f74d920e0637ea41b3289f5b55b4 incorrectly copies a GCC bug: https://sourceware.org/bugzilla/show_bug.cgi?id=15366 *and* overrides a #define that belongs to the libc/libc++ headers.
(The bug was the result of the GCC folks following an effort by the ISO C committee to tell the ISO C++ committee what to do. The ISO C++ committee ignored them, but the GCC folks didn't. The ISO C committee fixed their overstep in C11. See http://en.cppreference.com/w/Talk:cpp/types/integer.

Please remove that commit.

Don't change ChangeLog. It's automatically generated from the git log at each release.

You removed the code for the auto-read-only threshold.

Don't use Timespec in new code, use time64.

Don't remove function description comments; do make them Doxygen markup.

Don't use magic numbers. Create static const variables (not #defines!) with descriptive names.

> 
> The limits I quote again below still apply:
> 
>>> - The default timezone used for display *should* be specified in the
>>> environment variable TZ in the form "[^-+]*+HHMM$", HH and MM being hours
>>> and minutes to add to UTC times to get the local time (including the
>>> possible daylight saving time correction).
>>> 
>>> - These patches cannot be disabled by configuration (a patch to
>>> preferences.glade is lacking).
> 
>> What's the reason for using an environment variable for determining
>> timezone? TZ seems not commonly used and it would be trivially easy to add a
>> function to get the UTC offset in gnc-date.cpp.
> 
> This is still work to be done.
> 
>> Entry date handling has long been a matter of controversy in GnuCash, and
>> switching the nominal time to 11:00 UTC from 00:00 UTC is the simplest
>> option to resolve most of it. 11:00 instead of 12:00 because New Zealand's
>> summer time offset is +13 hours and the total population of the -12 timezone
>> is only a few thousand people. That change is already planned for GnuCash
>> 2.8 unless we decide to go in either of the directions Geert mentioned.
> 
> I have now adopted 1100Z as the default time on transaction creation. Time of
> loaded transaction are not changed (and they were midnight for gnucash-2.4*).
> 
>> My understanding is that it's unusual for accountants to care much about
>> exact transaction times,
> 
> This has been refered to in the thread "Rounding in the price db." about
> conversion rates, e.g., in the mail dated Wed, Aug 12, 2015 at 3:27 PM
> shown by gnucash.1415818.n4.nabble.com/Rounding-in-the-price-db-td4679860.html
> 
> I can provide references to other recent threads if it helps.
> 
> 2015-05-29 0:28 UTC+02:00, John Ralls <jralls at ceridwen.us>:
>>> I think that the coding and debugging style, the TZ environment variable
>>> and
>>> the lack of patch to preferences.glade are better reasons to disqualify
>>> it.
>> 
>> Those are all fixable. Once they're fixed, the patch can be considered for
>> master, though as Geert pointed out there's pressure in the other direction
>> to remove times entirely from Entered Date. We have to decide which way to
>> go.
> 
> The coding style is still problematic; and I did not use the recent
> Gnc_Date_Time class, short of time to test it.
> 
> * the one character string ":". In that case, it is replaced by
> the current time and date.
> 
> * Any entered date string starting by ":" and longer that ":" will crash
> * gnucash.
> 
> * if TZ is not defined, gnucash crashes immediately.
The crashes alone make this patch incomplete and unacceptable as-written. 


That's all I have time for now, and I won't have more for a couple of weeks. In that time, please rewrite your submission taking the above into account and complying with http://wiki.gnucash.org/wiki/CodingStandard. Put it in a feature branch and squash out irrelevant commits. I'll do a deeper review then.

But we still haven't decided whether to drop times altogether from date_posted, and this change, even if otherwise perfect, can't merge until we do.

Regards,
John Ralls