MFEK / glif

A stand-alone glyph viewer and editor (UFO .glif). (ꞵ quality)
Apache License 2.0
189 stars 14 forks source link

Save via git #114

Open davelab6 opened 3 years ago

davelab6 commented 3 years ago

While writing https://github.com/MFEK/glif/issues/103#issuecomment-848242931 I was reminded that most editors with good plugins get a "replace save() with git.commit()" plugin, and it's possibly so useful to be a good candidate for an out of the box feature.

Lmk if you want links to prior art, but it's a fairly simple concept.

Still, it can go deeper, such as adding auto save with auto commit message, dialog to capture commit messages, advanced UI for file and even line selection, stash, branching and tagging, push/pull.

Possibly this is related to the inotify hot reload of UFO stuff, since checking out another commit will trigger that, etc

ctrlcctrlv commented 3 years ago

Actually, rather than being a good candidate for an out of the box feature, I think this is a good candidate for an example evcxr script, because I want to add Rust scripting to both our headless mode and graphical mode. I hope to end up distributing example scripts in contrib/ similar to FontForge (except, functional, and no arguing this time about which ones go in because I'm charge! :stuck_out_tongue_closed_eyes:).

It'd just require hooking into the save hook and calling a shell command. Very simple example script.

davelab6 commented 3 years ago

I don't object to this, but I also think it will be very slow.

alerque commented 3 years ago

Which part of that suggestion do you think would be slow? Ironically even pre-compiled Rust using library calls to libgit2 to manipulate repositories is slower that shelling out from Rust to the system git command for many operations (especially the big batch operations such as 'commit all changed files' because it has optimizations that the library version does not) — and is only faster in some cases (usually if you want to round-trip a lot of times between your app and the Git object storage for blobs that don't need computing, just read/write). I don't see how executing this from a save hook would be appreciably different than making it an out of the box feature unless the spin up time for evcxr processing is significant (that‌ I don't actually know).

MatthewBlanchard commented 3 years ago

Fred and I talked about this recently, and we're going to make this our first plugin. It's a good candidate for that as it's going to have to hook into a lot of places we're going to want to expose to the plugin system, and it's relatively uncomplicated.

There is a consideration to be made on the shell script end of things is that it requires git in PATH for Windows users. Does libgit2 also require an executable in PATH or is that functionality built-in to the library itself?

alerque commented 3 years ago

Anything built with libgit2 (the Rust crate is just a wrapper around the C bindings) does not require any executable or path at all, everything is baked into the library. This tends to have pros and cons vs. a shell callout that are not a clear win for either side, it's very circumstantial. I've done it both ways and would still use different ways for different use cases but experience has taught me my gut instinct isn't always right on which one will be best, I really have to map out what it will be used for by who.

Given that such a plugin will probably not need to trawl through history, the library route is promising. But I don't know how well that will play with plugins being "scripts". Will such a plugin have access to a precompiled library so it can use the API? Or will adding the plugin require a Rust toolchain? In that case a plugin that shells out to the system and doesn't require any building might be better.

MatthewBlanchard commented 3 years ago

That's a really good point I hadn't considered. If this is going to be an eval'd plugin that runs from a script instead of precompiled there'd be no way to include libgit2. Thinking more about it having git in your path is not a terrible requirement for Windows. Someone using git probably already has it from the official git installer, and if they don't they're a download and hitting "Next" about 10 times away.

We could even check for Windows and just display a pop-up with customized text for the platform on Linux tell the user to install git from their package manager, and on Win/Mac link to the appropriate binaries.

alerque commented 3 years ago

If I'm understanding correctly, the only way it could be done with the library and "script" plugins is to make access to all the necessary libgit2 functions into the app (even though unused) and hence expose them, no? I don't even know if that's possible, and if it is it might not be worth it. As you said having system git tooling is not that onerous (although lots of other tooling provides their own embedded ones, so it's not impossible that even people that work with it a lot don't have the client.

ctrlcctrlv commented 3 years ago

Uh, I was thinking of making the script just require that the system have a git binary. If you don't have a git binary in your PATH, why would you even want this function? Nothing about libgit2.

davelab6 commented 1 year ago

What's the status of this? :)

alerque commented 1 year ago

Uh, I was thinking of making the script just require that the system have a git binary. If you don't have a git binary in your PATH, why would you even want this function? Nothing about libgit2.

The use case logic is sound: if you don't have Git on your system anyway you likely don't need this feature at all. But the implementation is another matter: the git binary is primarily meant to be a user facing tool. It does have some "plumbing" commands as opposed to the "porcelain" ones most end users use, but those are primarily meant for parsing in other CLI programs. Using them as embedded system calls inside a GUI app starts causing problems fairly quickly. Been there done that. Using library bindings makes it way way more robust, easier to handle errors, etc.

That being said the libgit2 bindings are a bit of a pain, I recommend gitoxide as better alternative.