limetext / backend

Backend for LimeText
BSD 2-Clause "Simplified" License
538 stars 87 forks source link

Pull out the clipboard wrapper #117

Closed erbridge closed 8 years ago

erbridge commented 8 years ago

Also add a flag to track if the current contents was auto-expanded, required for full paste command functionality.

Note this is a breaking change which will stop the commands from working.

ricochet1k commented 8 years ago

Why the getter and setter functions? It seems like a standard Go interface might be a better fit, in addition to being simpler.

erbridge commented 8 years ago

Probably. Trying not to change too much at once, though, and currently, the frontends choose the clipboard backend to use, and provide the functions.

erbridge commented 8 years ago

@ricochet1k Do you think refactoring it into an interface is something that needs to happen before merging this?

ricochet1k commented 8 years ago

I think it makes more sense to make one breaking change, rather than two.

erbridge commented 8 years ago

They break different things, but yes, I see the logic.

erbridge commented 8 years ago

Better @ricochet1k?

ricochet1k commented 8 years ago

I like the interface implementation, but I don't like how the internal handling of text/autoExpanded is tied to the selected clipboard. Shouldn't the clipboard only be responsible for getting/setting the system clipboard, not remembering internal clipboard stuff?

erbridge commented 8 years ago

It's conceivable that one clipboard implementation would manage to store autoExpanded on the system clipboard itself by structuring the data, allowing that information to be transferred between instances of Lime (e.g. between VMs using shared clipboards), for example.

Alternatively, where would you have Lime remember the internal clipboard stuff?

erbridge commented 8 years ago

I'm going to merge this. We can refactor it further later, if we want, but this enables functionality, which I'd like to get in.

zoli commented 8 years ago

LGTM, Also we should change the frontend remove the setting clipboard code.

erbridge commented 8 years ago

Yup. See https://github.com/limetext/lime-qml/issues/45.