-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add clipboard support #13837
Add clipboard support #13837
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -402,6 +402,12 @@ Remember to quote string arguments in input.conf (see `Flat command syntax`_). | |
Like all input command parameters, the filename is subject to property | ||
expansion as described in `Property Expansion`_. | ||
|
||
``screenshot-to-clipboard [<flags>]`` | ||
Take a screenshot and save it to the system clipboard. | ||
|
||
The ``flags`` argument is like the first argument to ``screenshot`` and | ||
supports ``subtitles``, ``video``, ``window``. | ||
|
||
``playlist-next <flags>`` | ||
Go to the next entry on the playlist. | ||
|
||
|
@@ -3573,6 +3579,35 @@ Property list | |
``current-ao`` | ||
Current audio output driver (name as used with ``--ao``). | ||
|
||
``clipboard`` (RW) | ||
Access to the system clipboard as a key/value map of types and data. | ||
|
||
Sub-paths can be accessed directly; e.g. ``clipboard/text`` can be read, written, | ||
or observed. | ||
|
||
Writing a string to the root ``clipboard`` property will write to the text clipboard | ||
as a shortcut. | ||
|
||
Converting this property to a string will give a JSON representation of all types. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do you plan to represent image data with this? Base64? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't expect raw image data to appear in the string representation; it'd only be useful to API clients interacting with binary directly (a la There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we have any other properties that do this? What's the intended use case for this behavor? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Everything that uses There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The manual doesn't mention this for any other properties so that might be what tripped me up. |
||
Unset types will be null. | ||
|
||
Writing to one type may cause other types to also be updated. | ||
|
||
Currently, the following types are available: | ||
|
||
``text`` | ||
Arbitrary plain text | ||
|
||
``url`` | ||
A URL; if ``path`` is populated, this will be a corresponding file:// URL | ||
|
||
``paths`` | ||
A list of absolute paths to files on disk. | ||
|
||
``path`` | ||
A single absolute path to a file on disk. | ||
Exposed for convenience and not included in the top-level map. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I could include it; it's just redundant with |
||
|
||
``user-data`` (RW) | ||
This is a recursive key/value map of arbitrary nodes shared between clients for | ||
general use (i.e. scripts, IPC clients, host applications, etc). | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -405,7 +405,8 @@ cocoa = dependency('appleframeworks', modules: ['Cocoa', 'IOKit', 'QuartzCore'], | |
features += {'cocoa': cocoa.found()} | ||
if features['cocoa'] | ||
dependencies += cocoa | ||
sources += files('osdep/language-mac.c', | ||
sources += files('osdep/clipboard-mac.m', | ||
'osdep/language-mac.c', | ||
'osdep/path-mac.m', | ||
'osdep/utils-mac.c', | ||
'osdep/mac/app_bridge.m') | ||
|
@@ -540,6 +541,10 @@ else | |
features += {'win32-smtc': false} | ||
endif | ||
|
||
if not features['cocoa'] | ||
sources += files('osdep/clipboard-dummy.c') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "one global handler" approach will not work on Linux. Both the X11 and Wayland code needs to be compiled in, but only one of them will be active depending on the VO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd expect to have a single linux implementation of the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Requiring only one implementation per platform sounds unnecessarily inflexible to me and as you proposed also leads to weird patterns where you have code that only forwards calls. Plus not being able to handle edge cases like X11 on macOS (just an example, not suggesting we should have that). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There can be multiple implementations of the underlying functionality per platform, they just need a implementation of the cross-platform APIs; if e.g. |
||
endif | ||
|
||
features += {'glob-posix': cc.has_function('glob', prefix: '#include <glob.h>')} | ||
|
||
features += {'glob-win32': win32 and not features['glob-posix']} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
/* | ||
* Dummy implementations of clipboard access routines | ||
* | ||
* This file is part of mpv. | ||
* | ||
* mpv is free software; you can redistribute it and/or | ||
* modify it under the terms of the GNU Lesser General Public | ||
* License as published by the Free Software Foundation; either | ||
* version 2.1 of the License, or (at your option) any later version. | ||
* | ||
* mpv is distributed in the hope that it will be useful, | ||
* but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
* GNU Lesser General Public License for more details. | ||
* | ||
* You should have received a copy of the GNU Lesser General Public | ||
* License along with mpv. If not, see <http://www.gnu.org/licenses/>. | ||
*/ | ||
|
||
#include "clipboard.h" | ||
|
||
int m_clipboard_set(struct MPContext *ctx, const struct m_clipboard_item* item) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also could we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not entirely clear on what the style guide situation is for when we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer to use |
||
{ | ||
return CLIPBOARD_FAILED; | ||
} | ||
|
||
int m_clipboard_get(struct MPContext *ctx, struct m_clipboard_item* item) | ||
{ | ||
return CLIPBOARD_FAILED; | ||
} | ||
|
||
bool m_clipboard_poll(struct MPContext *ctx) | ||
{ | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I definitely see the usefulness but maybe copying images is better left out of the initial implementation to keep it simple first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Screenshot support is pretty core to this functionality (it's one of, like, 2 main use-cases I'm targeting), so I'm hesitant to remove it unless we have a specific reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thought was that it might be better to have a fleshed-out API for the "copy text/URL(s) to the clipboard" first, merge that and then integrate image clipboards into that (in a different PR). Perhaps even in a way that can be used from the client API.