knqyf263 / pet

Simple command-line snippet manager
MIT License
4.47k stars 226 forks source link

Sync Improvement - merge snippets instead of overwrite #57

Open brpaz opened 6 years ago

brpaz commented 6 years ago

Hi. First of all, congratulations for building this tool I am using it everyday.

I added lots of snippets in the last days, and today I decided to run pet sync in order to synchronize to my snippets gist. But I didn´t specify the -u flag, so it downloaded the contents of the gist file and replaced all my newly created snippets! fortunately I had a backup. I was not aware of this behavior. I think sync should merge the contents of local and remote files and not replacing them like this. The current behavior is more of a backup, than a sync really. It I have pet running of two machines and add snippets In both of then, there is no way to sync automatically without losing data.

Hope this can be improved.

Keep up the good work!

kpron commented 6 years ago

+1

knqyf263 commented 6 years ago

@brpaz I'm sorry for the too late reply! I released v0.3.0. https://github.com/knqyf263/pet/releases/tag/v0.3.0

I changed the behavior of pet sync. pet sync compares the local file and gist with the update date and download or upload automatically. So -u option is deprecated. Please see https://github.com/knqyf263/pet#sync

In addition, auto_sync is added. https://github.com/knqyf263/pet#auto-sync

patrik-bartak commented 3 months ago

The sync feature is great, but I think there is still an issue with auto_sync that happened to me today. Let's say I have auto_sync on both machines A and B. I add a snippet on machine A and it syncs the gist. Now I add a snippet on machine B. It will add the snippet, making the local file newer, then upload, and overwrite the snippet added by machine A.

In my opinion, it makes more sense to do this when running pet new with auto_sync on:

RamiAwar commented 3 months ago

The sync feature is great, but I think there is still an issue with auto_sync that happened to me today. Let's say I have auto_sync on both machines A and B. I add a snippet on machine A and it syncs the gist. Now I add a snippet on machine B. It will add the snippet, making the local file newer, then upload, and overwrite the snippet added by machine A.

In my opinion, it makes more sense to do this when running pet new with auto_sync on:

  • sync with the gist to get any new commands
  • add the command
  • sync again to upload the command

You mean as in merge the changes?

patrik-bartak commented 3 months ago

@RamiAwar A merge is one option but not necessary.

I just mean it would be great if it didn't overwrite previously added commands from some other machine when I forget to pet sync before pet new. To achieve that it might be simpler to download (if remote is newer), add the command locally, then upload, no? With the same gist configured you can then add commands from any machine without touching pet sync with auto sync turned on, and none of them get lost.

I guess a question would be what would happen with pet edit. Would it also download first, then let you modify it, and then upload? Possibly, which to me sounds better than losing commands. If someone does not want this they can just disable auto sync or sync to different gists on different machines.

RamiAwar commented 3 months ago

@patrik-bartak sorry missed this.

Let me see if I understand correctly. So you're saying:

For 'merging' changes i.e. if you already have a local file, you're saying we compare modified at as well. (local file system modification vs sync client last update date)

patrik-bartak commented 3 months ago

@RamiAwar Yes that is what I had in mind. Do you agree this streamlines the use of pet, even if it is more complicated to implement? I imagine this will be useful for those that regularly switch computers.

RamiAwar commented 3 months ago

Yess I think it's definitely worth doing, makes sense!

patrik-bartak commented 3 months ago

The logic itself seems quite simple. Can I have a go at it or do you have time?

RamiAwar commented 3 months ago

Yess please do! 🙂 Don't have much time these days 😄

patrik-bartak commented 3 months ago

I'm on it. Can I link this issue or make a new one? I think this issue should have already been resolved.

RamiAwar commented 3 months ago

@patrik-bartak You can just use this issue! We can add more details in the PR description 🙂 Thanks!