Skip to content

Add Get::html on macOS (unimplemented on others)#157

Closed
fasterthanlime wants to merge 1 commit into1Password:masterfrom
fasterthanlime:get_html
Closed

Add Get::html on macOS (unimplemented on others)#157
fasterthanlime wants to merge 1 commit into1Password:masterfrom
fasterthanlime:get_html

Conversation

@fasterthanlime
Copy link

@fasterthanlime fasterthanlime commented Jul 11, 2024

So this is half-baked since it panics on non-macOS platforms, but I can work on finishing it up if that's something y'all are willing to bear the burden of maintaining long-term.

If not, it'll just live in a fork of mine! Let me know!

(also just noticing #130 now)

@complexspaces
Copy link
Member

Hey there, thanks for the PR. HTML is something we've committed to having it on the platform agnostic main API, so we'd be happy to wrap up that dangling thread by adding a matching get method to the crate. All of our supported platforms today (and future ones) can handle it just fine so I don't foresee any painful long-term maintenance 👍.

(also just noticing #130 now)

Yeah, its been a few months since that one had any activity. I think that my comment from that PR about where this API should live still feels valid, that its probably best to leave this functionality on the Get structure for now and consider adding it to the main clipboard API later if there's demand since its a non-breaking addition.

This crate doesn't have an explicit CLA, but I don't think it would be problematic to cherry-pick that PRs commit here and improve on it in more commits for merge if that's something that interests you vs writing X11 from scratch.

@fasterthanlime
Copy link
Author

I’m not sure how much time I will have to dedicate to this, but for what it’s worth I’m totally fine with this pull request being cannibalized by any further pull request that tackles all four target platforms at once!

@Gae24
Copy link
Contributor

Gae24 commented Oct 11, 2024

Hi, I was looking to get this done primarily on Linux, but I can also delve into how to implement it for other platforms too.
So isn't it a problem if I include your changes @fasterthanlime ?

@fasterthanlime
Copy link
Author

Hi, I was looking to get this done primarily on Linux, but I can also delve into how to implement it for other platforms too. So isn't it a problem if I include your changes @fasterthanlime ?

No problem at all, please do!

@complexspaces
Copy link
Member

With @Gae24's help this was merged via #163.

I'm going to close this as superseded, but thank you for your initial work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants