By default, Bugzilla does not search the list of RESOLVED bugs.
You can force it to do so by putting the upper-case word ALL in front of your search query, e.g.: ALL tdelibs
We recommend searching for bugs this way, as you may discover that your bug has already been resolved and fixed in a later release.
Bug 2691 - Proposal to extend the dcop interface for knotes
Summary: Proposal to extend the dcop interface for knotes
Status: RESOLVED FIXED
Alias: None
Product: TDE
Classification: Unclassified
Component: tdepim (show other bugs)
Version: R14.1.x [Trinity]
Hardware: All Linux
: P5 normal
Assignee: Michele Calgaro
URL:
Depends on:
Blocks: R14.1.0
  Show dependency treegraph
 
Reported: 2016-09-14 15:48 CDT by deloptes
Modified: 2020-02-29 07:21 CST (History)
5 users (show)

See Also:
Compiler Version:
TDE Version String:
Application Version:
Application Name:


Attachments
patch proposal to extend the knotes interface with getRevision() method (4.40 KB, application/octet_stream)
2016-09-14 15:48 CDT, deloptes
Details
patch proposal to extend the knotes interface and fix for the LAST-MODIFIED field (6.63 KB, patch)
2016-09-19 10:12 CDT, deloptes
Details | Diff
patch proposal to extend the knotes interface and fix for the LAST-MODIFIED field in knotes and kontact notes part (13.88 KB, patch)
2016-09-20 08:47 CDT, deloptes
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description deloptes 2016-09-14 15:48:51 CDT
Created attachment 2689 [details]
patch proposal to extend the knotes interface with getRevision() method

I want to herewith propose extending the dcop knotes interface with a getRevision() method.
This would be of great advantage for synchronization. 

Patches included.

Let me know what you think

regards
Comment 1 deloptes 2016-09-15 01:37:32 CDT
This seems to be not enough. After testing it in "production" I noticed the revision is updated each time knotes program is stopped. It looks like it writes all the notes in the file each time it goes down and revision is updated.
Comment 2 deloptes 2016-09-16 03:34:19 CDT
I change the priority to normal from enhance, because it  does not look like normal behavior to change the LAST-MODIFIED field when journal was not modified.
Comment 3 deloptes 2016-09-17 17:31:24 CDT
I traced it down to m_editor->isModified() returning true, even if item was not modified.

Test to reproduce this

bool KNote::isModified() const
{
   if (m_editor->isModified())
	kdDebug(5500) << "KNote::isModified()  m_editor->isModified() false"<< endl;
   else
	kdDebug(5500) << "KNote::isModified()  m_editor->isModified() true"<< endl;
    return m_editor->isModified();
}

1. start knotes
2. close a note (being opened) => returns
  knotes: KNote::isModified()  m_editor->isModified() true
3. close knotes => returns for each item
  knotes: KNote::isModified()  m_editor->isModified() true

This is defined in
  /usr/include/tqt3/ntqtextedit.h

to be continued
Comment 4 Michele Calgaro 2016-09-17 21:09:48 CDT
Hi Emanoil,
it is not a problem to extend dcop interface for a program if the extension is a good addition ;-)
Just note that any extension will go to r14.1.x and not r14.0.x though.
Comment 5 deloptes 2016-09-18 03:52:15 CDT
Thank you Michele. 
I raised it against 14.1, so I am aware of this. I would rename the function from getRevision to getLastModified as revision is confusing with SEQUENCE. Until 14.1 I will rebuild from git as I did now.
My problem ATM is why it always updates the LAST-MODIFIED field as mentioned in the bug - editor->isModified returns true.
I'll try to find a better solution and update the patches
Comment 6 deloptes 2016-09-19 01:40:23 CDT
(In reply to deloptes from comment #3)
> I traced it down to m_editor->isModified() returning true, even if item was
> not modified.
> 
> Test to reproduce this
> 
> bool KNote::isModified() const
> {
>    if (m_editor->isModified())
> 	kdDebug(5500) << "KNote::isModified()  m_editor->isModified() false"<< endl;
>    else
> 	kdDebug(5500) << "KNote::isModified()  m_editor->isModified() true"<< endl;
>     return m_editor->isModified();
> }
> 
> 1. start knotes
> 2. close a note (being opened) => returns
>   knotes: KNote::isModified()  m_editor->isModified() true
> 3. close knotes => returns for each item
>   knotes: KNote::isModified()  m_editor->isModified() true
> 
> This is defined in
>   /usr/include/tqt3/ntqtextedit.h
> 
> to be continued

Hi my example is obviously wrong, thus the conclusion that m_editor->isModified() returns true

It should be

bool KNote::isModified() const
 {
    if (m_editor->isModified())
 	kdDebug(5500) << "KNote::isModified()  m_editor->isModified()  true"<< endl;
    else
 	kdDebug(5500) << "KNote::isModified()  m_editor->isModified() false"<< endl;
     return m_editor->isModified();
 }

It looks like the LAST-MODIFIED field gets populated internally when calendar/journal is loaded.
On the first write it does overwrite all LAST-MODIFIED.
Comment 7 deloptes 2016-09-19 10:12:53 CDT
Created attachment 2690 [details]
patch proposal to extend the knotes interface and fix for the LAST-MODIFIED field

After getting formal OK from Michele I updated the code (instead getRevision I added a method getLastModified). Also I added a fix for the setting of LAST-MODIFIED to current timestamp when saving notes.

The same needs to be done for the knotes_part in kontact, a patch will follow.
Comment 8 deloptes 2016-09-20 08:47:51 CDT
Created attachment 2691 [details]
patch proposal to extend the knotes interface and fix for the LAST-MODIFIED field in knotes and kontact notes part

Attached what I believe to be final patch for now.
As before extended the interface to be able to get the last modified
and both knotes and knotes_part in kontact can work on notes without loosing track of the modification time stamp.

regards
Comment 9 deloptes 2016-11-20 10:07:52 CST
an update
This works extremely well. Syncing notes is very reliable so far.
Should I push the change or was it applied already?
Comment 10 Michele Calgaro 2016-11-21 07:52:04 CST
Not applied yet, AFAIK.
Let me discuss with Slavek during the coming weekend (possibly).
And again thanks for your contribution ;-)
Comment 11 deloptes 2017-05-02 06:24:49 CDT
still nothing done here? Perhaps we could arrange a virtual meeting and have a discussion on pros and cons.
I would say it is important for syncevolution to work (with my plugins)
Comment 12 Michele Calgaro 2018-10-07 07:04:29 CDT
Emanoil,
can you create a PR on gitea for this? it will be easier to review, to feedback, to modify (if required) and to merge to the main trunk. I would like to finally work on this.
Thanks.
Comment 13 Michele Calgaro 2018-10-07 07:05:57 CDT
Also what be a good way to test this (before and after)?
Comment 14 deloptes 2018-10-07 12:54:55 CDT
I would love to but at the moment I still do not know how to work with Gitea. I did not have the time to read the introduction and so on and my git knowledge is anyway limited.
I simply need to catch up, but time is an issue, so if you are more specific and point me to a link where I could read how to create PR (I guess pull request), it will be nice, otherwise I will look into that in the next week.

regards
Comment 15 deloptes 2018-10-07 12:58:12 CDT
And BTW I am using since I created the request. I do not see any disadvantage, but you can decide for yourself perhaps after testing with all those different flavors etc.
It made use of syncevolution very robust - zero issues while syncing all those years.

regards
Comment 16 Michele Calgaro 2018-10-08 06:33:01 CDT
Hi Emanoil,
see this section of the gitea guide. This is specific for contributors and how to work with PR. As long as your are cloning your repo from gitea (e.g. "origin" points to gitea repo and not main server repo), it should work straight away. 
https://wiki.trinitydesktop.org/TDE_Gitea_Workspace#To_contribute_code_changes

It is good to know you have been using the code for quite a while, it means it is robust :-)
Comment 18 Michele Calgaro 2020-02-29 07:21:09 CST
The extension of DCOP interface has been merged to R14.1 development branch, so we can consider this bug closed.
Some more work is still required to fix some issues with update of the last modified field in knotes, but this is handled in a separate issue.
https://mirror.git.trinitydesktop.org/gitea/TDE/tdepim/issues/38