helm-backup icon indicating copy to clipboard operation
helm-backup copied to clipboard

Some improvements and further thoughts

Open ghost opened this issue 10 years ago • 19 comments

Hello,

I created a fork of your excellent library to adopt it to my needs; here: https://github.com/michael-heerdegen/helm-backup. It was a joy to work with your stuff.

This is a work in progress, some variables still miss defcustom definitions and such stuff, but it is ready to try.

Would you want to work with me to include this stuff? Here is a list of important changes:

  • Drop dependency on "s". We used only one trivial function from "s", so it is overblown to depend on it.
  • Migrate to cl-lib.
  • By default, don't copy files into the backup repo but use hardlinks. This makes it possible for the user to switch into the repository and manipulate the file from there with git commands.
  • Instead of using porcelain commands and branches, use plumbing commands and plain references. This allows for more flexibility; OTOH, we didn't really make use of any branch features.
  • Don't hardcode the path to git but use `executable-find'.
  • By default, show commit message in Helm `helm-backup' because it includes useful info now (see below).
  • New var `magit-backup-respect-git-branches'. When non-nil, for files versionized with git, backups made with different branches checked out will backup into different histories, to avoid leaps in the backup history that just reflect the differences between branches.
  • Simplify semantic of `helm-backup-exec-git-command' for better readability.
  • Use file name manipulation functions instead of string operations to manipulate file names
  • Add global minor mode `helm-backup-mode'. Related improvements:
    • When appropriate, a backup is now also being made before saving a file. This gives the user a backup of the original version of a file when editing it for the first time or after it had been changed from outside Emacs. This is only done when necessary.
    • The first save of a file in an Emacs session will add "(new session)" to the commit message.
  • Avoid using a shell when calling git by using call-process-shell-command'; instead useprocess-file'.
  • `helm-backup-versioning' is now a command to allow for explicit backups with dedicated messages. This is for better orientation; if you have finished some changes, you can make a backup with a message "Finished changing this and that, works so far" and can find it easily later via the helm interface. That even works when there are no new changes.
  • New command `magit-backup-magit-log' gives you a Magit log of the backups of the current file. With an active region, it shows only commits related to the marked lines.

If you don't have time or don't want to go into that direction, I could also develop my fork on my own into something separate.

Apart from these changes, I would like to split the thing into two libraries:

  • a standalone library with the backup stuff, independent from helm
  • a library for the helm interface. i plan to extend the helm interface with other useful stuff, so that you can also diff against real commit or the index, and also to manipulate the index.

Thanks in advance and with kind regards,

Michael.

ghost avatar Sep 11 '15 10:09 ghost

Hi Michael,

thanks for your message and your interest, I'm going to answer at every point you mentionned and have a a look to what you've done. Just a thing, it would have been easier to review if you have made one commit per change and a pull request instead of one big commit.

antham avatar Sep 13 '15 16:09 antham

antham [email protected] writes:

thanks for your message and your interest, I'm going to answer at every point you mentionned and have a a look to what you've done. Just a thing, it would have been easier to review if you have made one commit per change and a pull request instead of one big commit.

Yes, I know. I made the changes so quickly, and they overlap so much, that it would be a some work to separate them into independent commits.

And note: I just wanted to give you an overview of what I did, and wanted to know whether you are, in general, interested into going into the direction I suggested.

If you are interested to merge my changes, I will of course split into commits and give you pull requests etc. I just wanted to avoid unnecessary work in case you are not interested or don't have time. The current code is not even completely finished (some defcustoms and docstrings missing etc).

So, just have a look, and tell me how you prefer to continue.

Regards,

Michael.

michael-heerdegen avatar Sep 13 '15 16:09 michael-heerdegen

I understood, after writing this comment, I was thinking you did so, cause you expected my opinion first. Having a quick sight, I'm 100% agree about your point concerning git plumbing, I did it quickly and you are right we only need storage and tracking capabilities of git, nothing more. About dependencies, concerning cl-lib, not knowing lisp ecosystem very well, I had a look about that on google and I'm ok with that change as well. About s, your argument is correct, but using good external library to avoid adding such trivial function and keeping them aside from core considering we have now a package manager in emacs, seems valid as well to me. Here I would say it's really a matter of taste except if you have a good argument against that, so if you really want to remove it, I have nothing against that.

Let me some time to review carefully other points.

antham avatar Sep 13 '15 17:09 antham

Ok, I'll send you orthogonal PRs successively. Let's start simple: https://github.com/antham/helm-backup/pull/13

Note: I don't know anything about the other files in the repo besides elisp and git ones. If I need to know something about them to make the PRs and avoid to break something, let me know. And I don't want to update the README in between until we are more or less done.

Regards,

Michael.

michael-heerdegen avatar Sep 25 '15 23:09 michael-heerdegen

Some notes about ongoing progress: I changed my mind and dismissed the idea of saving under separate refs when having different branches checked out. This is more confusing than helpful. Instead, I now just put the branch name into the commit message.

And I'm currently trying to use the file's own repository for our purpose when it is git versionized. This is possible since I converted to plumbing commands. And doing that, all our commits are valid commit objects in the original repository, so you have access to them via git in that directory. That allows you to git-diff the head of a different branch with an older backup save, for example.

All this will become a PR in the near future. Just want to let you know in case you think "No, not with me!" or so.

michael-heerdegen avatar Sep 26 '15 03:09 michael-heerdegen

Sorry for this big delay, I wasn't free those days, I will finish to review everything today. Thanks for your messages, I merged your PR.

antham avatar Sep 29 '15 06:09 antham

antham [email protected] writes:

Sorry for this big delay, I wasn't free those days, I will finish to review everything today. Thanks for your messages, I merge your PR.

There is no hurry, as long as we don't get absolutely stalled (and the risk for that is large from my side, too). If we don't completely forget about it, I don't mind much how long it takes.

michael-heerdegen avatar Sep 29 '15 21:09 michael-heerdegen

Ok great. About other files in repo, it's related to functional test and to have a development environment so they are important, you can have a look here https://travis-ci.org/antham/helm-backup to see how it works.

antham avatar Sep 29 '15 22:09 antham

Here is the next one: https://github.com/antham/helm-backup/pull/14.

michael-heerdegen avatar Oct 01 '15 01:10 michael-heerdegen

Are you still working on all of these chages/features?

humitos avatar Jun 18 '17 20:06 humitos

I don't he does @humitos it's an old ticket.

antham avatar Jun 20 '17 19:06 antham

Anthony HAMON [email protected] writes:

I don't he does @humitos it's an old ticket.

Actually, I developed my own version of the package all the time, but was to lazy to invest the time to merge. I think I could still do it, but that would need some time, and, maybe, help.

michael-heerdegen avatar Jun 21 '17 02:06 michael-heerdegen

Yep it's partly my fault, I wasn't helpful about that, I will have a look this week-end to see what can I do about to start to pick in main repository some features.

antham avatar Jun 21 '17 07:06 antham

Anthony HAMON [email protected] writes:

Yep it's partly my fault, I wasn't helpful about that, I will have a look this week-end to see what can I do about to start to pick in main repository some features.

My fault is that I stopped uploading to my fork on github some while ago. I have to look how much our two versions diverged, and if rebasing my local version to your repo has become more complicated. But it's nothing we couldn't handle I guess ;-)

michael-heerdegen avatar Jun 21 '17 07:06 michael-heerdegen

It doesn't change from the time you did it but it has to be included step by step with some tests and I have to check what we keep and what we leave (if we have to leave some stuffs)

antham avatar Jun 21 '17 08:06 antham

@michael-heerdegen I like most changes of your version. Can you try and make some PRs or can I cherry-pick some of your stuff and I would then make PRs here? If you have a newer version locally and not on github, please push so I can check it out.

Thanks.

dakra avatar Dec 05 '17 09:12 dakra

Daniel Kraus [email protected] writes:

@michael-heerdegen I like most changes of your version. Can you try and make some PRs or can I cherry-pick some of your stuff and I would then make PRs here? If you have a newer version locally and not on github, please push so I can check it out.

Sorry for the late answer...

I made a bunch of changes in the meantime, but more in a rush and for myself. It's tested (well tested, by myself), but some stuff is half-baked.

I can upload the stuff if you want - but what I really want...is to start from scratch to make the thing independent from Helm (because it's useful in general) - preferably in Gnu Elpa - and make helm-backup a Helm interface to the new thing. It's on my list, but it's far from the top... Anyway, if you think you can help in any way, that would be very welcome, but I can not invest that much time myself right now.

Michael.

michael-heerdegen avatar Dec 08 '17 21:12 michael-heerdegen

Michael Heerdegen [email protected] writes:

I can upload the stuff if you want

I have uploaded the version of the file I am currently using myself to a branch named "current" in my fork.

michael-heerdegen avatar Dec 13 '17 13:12 michael-heerdegen

I have uploaded the version of the file I am currently using myself to a branch named "current" in my fork.

Nice, thanks.

dakra avatar Dec 13 '17 14:12 dakra