-
Notifications
You must be signed in to change notification settings - Fork 37
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
Tomdroid sync, improvements #141
Comments
And missing the super convenient sync with nextcloud (take the code from old Tomboy) |
Cannot take existing code from old Tomboy, its written in Mono (a C# dialect) and the whole rewrite was to remove the dependency on Mono and a huge stack of GTK+ stuff. Unfortunately, the network sync protocol itself is poorly documented and based around OAuth v1, which is getting something of a bad name. Thanks for your input grosjo, without this feedback I canny tell what end users really want. |
The huge benefit of the old tomboy (desktop and android) is the sync of notes with Nextcloud. It uses the work from @cweiske : https://github.com/cweiske/grauphel which is well documented. Probably worth it to talk with @cweiske about it ? |
There was one security problem in OAuth v1, which was fixed with OAuth v1.0c. There is no security problem anymore in OAuth v1, and thus no reason not to use it.
I found documented well enough to implement the server (grauphel). |
Yes, thanks Christian. However, I found while implementing file sync, there are a number of anomalies in that documentation. I believe what probably happened is that the Tomboy developers wrote the document you mention and then started coding. Found some corner case problems and implemented appropriate fixes. But did not go back and update the document. So, while I do, honestly believe tomboy-ng is compatible with a Tomboy file sync'ed repo, it does work differently. And that difference is significant. In particular, the tomboy-ng model is much safer IMHO ! I have implemented the sync system in a rigid object model and, for example, found it quite easy to add a new transport layer to add the one on one Tomdroid sync. To add a network sync, all I would need to do is understanding OAuth and your transport, but am not sure I am up to that ! Nor would I be confident that those corner cases would not put user's data at risk. The decision process is amazing (IMHO) complicated. You can see my feeble effort to document it on the wiki here. However, I am not happy I have captured everything there even. Sigh ... Davo |
I would be able to provide a PR for that "sync" capability, but need to remember the "Pascal" syntax first :D |
That is an REST-API to edit notes. The Tomboy Sync API is more efficient because it makes it easier to find out which notes have not been synced. |
@cweiske Does your server-side API interact with Nexcloud Notes (stock) ? |
No. It is just a sync API for clients, with read-only access in the nextcloud web interface |
Is it a big work to connect it to the nextcloud notes (where it has already a web interface and an android app) ? |
Tomboy's note format is clearly defined (bold, italic, sizes, lists), and I have no idea what is allowed in nextcloud notes - but I guess it has more formatting options. This means that there will be conversion problems. |
I will look at that during the weekend and let you know if I am able to provide a PR |
@davidbannon I managed to install Pascal and compile Tomboy-ng |
Probably transnet.pas, replacing the call to Ruby by REST call on the network ? |
Wow, that's is great news in a world starved of good news right now ! Joan, sounds like you are on the right track. You have fond TNetSync = Class(TTomboyTrans), you could either hack into that fire directly, it was started by Ben but he was distracted. Or you could inheritance a new class of your own, maybe TNextSync = Class(TTomboyTrans) ? If you have something working, I can easily make it available from the sync class. Anyway, you can see the functions that need to be implemented. My guess is that would not be too hard, the part that worries me is the security layer. I think it should all exist in in the Trans class but I don't understand enough of the OAuth model to say how. The old Tomboy would, at first contact, trigger a web browser session in which you established credentials. That, presumably left a key of some sort that could be used in subsequent sessions. Chris would be able to correct my ramblings here I am sure. So, maybe the new class reads and writes directly to that key, it can easily ask the Settings unit for the right place to do that. Or I can extend the Settings unit to handle the I/O. As I understand it, we would not save the user's actual password to disk at any stage. Chris, could you please help here ? Its no secret I do not understand OAuth ..... The actual file movements, the gets and puts should not be that hard. I am only too keen to help you there..... Davo |
Can you clariify the following function (and the associated parameters) function TestTransport(const WriteNewServerID : boolean = False): TSyncAvailable; override; function DoRemoteManifest(const RemoteManifest : string) : boolean; override; Thanks |
TestTransport() in file sync just checks that the expected config dir contains things that look like a sync repository. For example, if the drive that contains the repository is not mounted, it will make sure we abort the sync process. If the Parameter WriteNewServerID is true it will not worry about finding repo files there but does make sure things are writeable and will 'initialise' a new repository. It returns an 'enumerated type', one of several values, one of which is OK, the rest are one way or another error codes. DoRemoteManifest() In Tomboy speak, a manifest is an xml file containg things like a list of all relevent files. The remote manifest is a file in the remote sync repository that lists all the notes that are available in the repository. Every time you add a new file, that is, sync a a local file so that its copied up to the remote repository, the remote manifest must be updated with information about the new file and stamped with a new version number. In file sync, because the remote manifest is mounted locally, all that I do is overwrite it with one containing the new data. In a network sync, I guess we write it out locally to disk and then send it via a 'post'. Or perhaps a post via a stream, direct from the data structure in memory ? |
Ok |
Wow, you are not messing around are you ? I will sit down and have a good look at it later today ! |
The PR does nothing else than adding an empty class so i have space to work. The other issue is the settings : Can you tell on thid one ? Will uou update it or shall I do it ? (url, login, pass). How to retrieve those parameters in in the TSync class ? A challenge I foresee is how store credentials securely, using Gnone Keyring most probably Do you know the Pascal way of doing it ? |
OK, pull merged. I'll put a define thingo in the code to show a network sync option in Settings if its defined. Davo |
Strange, I cannot find this message in github.
anyway, answering direct from my email client. Sadly, no, the LCL GTK3
interface is very incomplete. I have played with it a bit, and submitted
a couple of patches but its a long way from usable.
QT5 on the other hand is pretty good. Especially if you use the Trunk
version of Lazarus. And the QT5 libraries are about 20% of the size of
GTK2. So, if you have a new system with just GTK3, QT5 is a better bet.
There is almost no difference in using different widget sets in
Lazarus, thats its main feature IMHO, the ease of making something work
on any supported platform.
Davo
…On 8/4/20 5:16 pm, Joan Moreau wrote:
Shall that be usefull to access the GTK function ?
https://wiki.freepascal.org/Gtk%2B3
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#141 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABP76MBDCTOSVECEY5UF45TRLQQEBANCNFSM4KVWOUZQ>.
|
By the way, why did you use KControl and not Gnome or QT ? |
OK, I just checked in some support there for SyncNextCloud. A define in Settings, just under start of Implementation section controls it. KControls is a completely different sort of thing from Gnome or Qt. KMemo (part of KControls) is a component. Components use whatever widget set is selected (if necessary) to display themselves. Generally widgets are an operating specific thing, we have Cocoa on the Mac, Win32 on Windows and GTK2 or GTK3 on Linux. But Qt5 can be used on all all three. And, while wildly unsuccessful, GTK2 is available on Windows with some pretty hard to install libraries and was even once usable on the Mac ! As a matter of interest, we could have used a Component called RichMemo instead of KMemo but its quite incomplete, getting better but has a long way to go before its a KMemo replacement. Its a lot lighter, small component and an easier install. Every now and again, I have a look at it, send in a few patches. Davo |
Oh, and I should have said, nice work too Joan, have you worked with Object Pascal before ? Davo |
no, but copy-paste is a good helper in software ;) |
I already git pull, I don't find where are the network settings (URL, login, pass)
In the app itself, it does not appear (cf screenshot)
Can you give me a pointer ?
|
I understand, the logic shall probably be a bit different Therefore, |
AutoSync has been moved, only on the form, because it is not something the user chooses when there is a sync clash. Previously it was below the sync clash section and appeared to be a new section. But you moved it up and to the right where it appeared to be directly under the heading about sync clash. So, it had to go somewhere, the Sync block makes sense. The main, sync block is almost exactly the layout you did, only change is there is now one button that changes whichever sync model is selected. We can only have one sort of sync active at the same time, we need to make it plain to end users that they need to choose one or another. And I need to disable the nextcloud while shipping the next version, so less thing that need to change, the better. Behind that one setup/change button, you will find your code, unchanged. Specifically what logic appears upside down ? I am sure we can fix whatever is worrying you. I will try and do a merge if you like, its only an issue in Settings.pas and git does make the merge process relatively easy. Davo |
I will issue a PR to fixup , to avoid losing the work done at least |
I am quite happy to try to merge the existing one, most of what you have done will be easy. I have merged far bigger conflicts than that one ! |
Oh, you have closed it ! Most of the merge issues there are not relevant to you, they are chages I have made between your checkouts. And things like the translation files that are auto generated in the IDE. And trivial changes such as form position because one of us has moved a form on screen. So, manageable. |
I created a clean PR
|
@davidbannon Creating a "branch" for NC will add more complexity, not really willing to go that way. We need to keep this simple , so I can finalize the first release this weekend. |
@davidbannon I put "not" in from of the define to have the NC part, and my work is again gone again. This is depressing |
No, thats not all thats happening there. You are seeing the old form, just when we started messing around. You must be compiling the wrong code block. I note you have both a master and NC (?) branch in your tomboy-ng repo, are you sure you are using the right branch ? Please check your code base. (if something has really gone wrong, check out my 'next' branch, it has what you are expecting and I can switch between the NC enabled / disabled reliably.) Davo |
I am sure you know this but just in case - git branch [enter] // show active branch Davo |
@davidbannon What difference you do between "JoinSync" and "ManualSync" ? |
@davidbannon Question 2 : "GetNewNotes" -> THe code for file sync seems getting /ALL/ notes, whereas the function names seems to requires /ONLY NEW/ notes. What is the exact expected function ? |
@davidbannon Question 3 : Why do you separate RepoNew, RepoJoin et RepoUse ? What are the use cases ? This is totally not consistent with Nextcloud, and also FileSync. ALso, it is incosustent with JoinSync vs ManualSYnc (according to the code) Can I reshape all this properly ? |
{ Called when user wants to join a (possibly uninitialised) Repo, will handle some problems with user's help. } { Called to do a sync assuming its all setup. Any problem is fatal } |
function GetNewNotes(const NoteMeta : TNoteInfoList; const GetLCD : boolean ) : boolean; virtual; abstract; Question 2 : "GetNewNotes" -> THe code for file sync seems getting /ALL/ notes, whereas the function names seems to requires /ONLY NEW/ notes.
It asks for a list of the notes the server knows about. The LCD stuff is probably not available in nextcloud, its an optional capability I added to make it easy to avoid Tomboy's habit of generating half a million duplicate notes. In the Tomdroid sync model, that again cannot include the LCD in the manifest, I pull down the note, look up the LCD myself. |
See text at top of sync.pas - it explains how the three modes work.
I believe its consistent with the Tomboy sync model.
It is most certainly consistent with FileSync. Its used against FileSync repos that are also used bt Tomboy. It you think that is not the case, please indicate in what manner.
No, or at least not unless you can demonstrate some problem with it. Please see the flow chart I pointed you to on the -ng wiki. |
If you refuse alignemnet of your alpha-release work to something a bit better, keep going |
If you do agree to receive criticism without beeing offended every line , then : 1 - "GetNewNotes" shall be renamed "GetAllNotes" 2 - if "JoinSync" is a dry run, then let's call it a dry run ("DrySync" for instance 3 - The status RepoJoin/RepoNew/RepoUSe is not aligned with the concept of DryRun (or call it "joinRun" incorrectly). THis is not working for now. The third is blocking. Etiher I will try again to bring value to your software (and you accept that you do /not/ have the unique truth), or I just give up |
I am sorry, I don't understand that line. If you are referring to my basing my recent release on a different branch, I absolutely have no choice. Your new code is alpha, its un tested and has a number of things that i already know, just by browsing through it, will break. If I edit it to fix some of those things, you just resubmit them. I cannot work that way. You submit huge pull requests that I find are impossible to review and reject the ones I know need to reject. I absolutely have to keep alpha code separate from the mainstream ready for release code, there is no project anywhere except perhaps a first year prac assignment where you do no practice change control. You are submitting alpha code, if you can get what you want to work, then together we can incorporate your new ideas into the mainstream product. Or we will switch to your stream when its mature. I will keep a log of the bug fixes from now until you have something working and, if I can, and you allow me, I will apply those bug fixes to your branch, what ever it is. And i will build and test your branch on the other platforms. But that gets harder and harder as you make changes that have absolutely nothing to do with what you are trying to achieve. If you want to be successful in software development, you must be prepared to work in a team and work under the constraints that that team has to work under. You need to focus on the assigned area. You cannot walk into a an existing project and apply your ideas all over the place. Please do apply them in the areas you have undertaken (and been welcomed ) to work on but changing other arbitrary bits of code that you don't understand yet is impolite and very likely to create hostility. Your code, what little I have seen, is good. But your understanding of how to work in a team is lacking. Over the years I have worked in a lot of teams, I have lead large (30+) teams of developers and sysadmins and I would like to help you learn the necessary skills. You, me and this project would benefit. A developer is not just someone writes good code, you may well already be a better coder than me. But a developer can achieve what is needed without putting the project at risk. Davo |
Firstly, you need to understand the differences between words such as shall, should, must, can, may.
JoinSync is not just a dry run, its the act of joining a sync, thats why its called JoinSync.
That is a statement, not a question nor an argument. I disagree with the statement, if you wish to make an argument that it needs to be changed, please do so, I will read it in full. Davo |
Yes it is a statement. I have to reshape properly the sync process. ok ? |
I have merged your current pull request with the 'RELEASE branch as requested. But please note that I must clean up this mess you have made with branch names. I have had complaints from a downloader who has checked out what he thought was the release code and got your release branch, unusable ! So, after I have merged this branch, I will remove all branches except develop and release (please do not reintroduce them back into the repo again!). Then, when you are ready, I will rename your branch to netsync and mine to master ? Is that OK ? After that rename, you must checkout and push to the netsync branch - do you understand ? I will proceed with this even if you ignore my request for an answer as usual. I have to do this. Do you understand ? David |
OK, I have removed the next, ref and master branches. Please remove them from your local repository to avoid accidentally reintroducing them again. git branch -d next [enter] I will proceed with the renaming when you acknowledge. David |
OK, I have merged that last revision. However, it did not go well, honestly, I don't know why not, but I have never merged such a big pull request in the past. If I have contributed to this problem, I am sincerely sorry but I don't think I did. But I do not know what went wrong, after the merge, the files in the repo were several weeks old ??? Fortunately, I kept a backup, restored from that and manually merged. I believe what is there is now correct but frankly, I am not sure. I strongly suggest you checkout to a new working repo and test. The repo where this work is happening is now in the netsync branch and the default branch is now, again, master. As it should be. Pull requests that target master or release will not be accepted. Renaming branches is always a risky operation and can, and should be avoided by people using the branch they have been assigned. David |
None of my changes have been merged :( |
I am closing this ticket, it was originally to remind me about some minor changes to the tomdroid unit but grosjo took it over. grosjo and I have decided we do not want to work together, I cannot handle grosjo's demands that he/she be allowed to submit alpha code to the master branch. My notes, and the notes of my end users are too important. Please see grosjo's github page for further developments. Alternativly, if you want to sync to a remote server using tomboy-ng, consider using sshfs. While away from home for extended periods of time, I sync to a ISP's server using sshfs and tomboy-ng's existing file sync. Trivial. David |
I'm interested in a synchronization Tomboy-ng <-> Nextcloud notes (without Grauphel which is not supported in the official Nextcloud docker image)
What's the URL of the branch or patch to look at? |
Monperrus, I would appreciate it if you could open a new issue rather that laboring this one. This thread has some bad memories :-) But, putting that aside, your question ? A sync with Nextcloud so that the tomboy notes are synced with Nextcloud's own note application ? That sounds pretty useful. I have designed the sync system of tomboy-ng so that it 'should' be easy to extend to other end points. Basically, it would be a a case of a unit that inherits from trans.pas. Thats an abstract class, it has no implementation, you derive a new class from it and implement there. In theory, it should mostly all be implemented there, nice to minimize changes to the rest of the tree. But clearly, some 'upstream' changes will be needed, I just need to avoid unexpected changes ! At present, there is only one branch, I'd be happy to create another branch for you to submit to. I'd ask you to be careful to submit to just that branch and, perhaps, discuss any significant changes outside the new 'nextcloud' class. ObjectPascal is happy to play with (eg) C libraries, if (and only if) you feel that would be a more familiar way to work, might be worth looking into. Davo |
oops, sorry for this, I understand. See #199 instead. |
The tomdroid sync needs a couple of tweaks -
not sure this will make it into next release....
Davo
The text was updated successfully, but these errors were encountered: