Subject: bug#76120: This feature is not about "sharing", or a "native" anything.

From: Alvaro Ramirez <[email-masked]>
Date: Thu, 20 Feb 2025 13:11:13 +0000

Visuwesh <[email-masked]> writes:

> [வியாழன் பிப்ரவரி 20, 2025] Alvaro Ramirez wrote:
>
> In interest of adapting my personal KDE Connect code as a 
> plug-in for
> the new library, I have some comments below.

Thanks for the comments Visuwesh!

KDE Connect sounds like a great option for KDE users! Would you be 
interested in submitting your code as a follow-up patch to include 
it amongst the known mechanisms?

>> +(require 'seq)
>
> This isn't required since seq is preloaded nowadays IIRC.

Great! Will remove.

>> +(defcustom send-to-support-checker-function 
>> #'send-to--default-support-checker-p
>> +  "A function returning non-nil when sending is supported by 
>> current platform."
>> +  :type '(function :tag "Function")
>> +  :group 'send-to
>> +  :version "31.1")
>
> Does this need to be a defcustom?  This seems like an internal 
> feature,
> and I wonder if a defvar/defcustom is necessary even.

If we want the feature to support configurable alternatives, we 
need to be able to check for required dependencies. This is just 
the customization point for such a check.

>> +(defcustom send-to-handler-function #'send-to--default-handler
>> +  "A function handling `send-to' external routing.
>> +
>> +The function receives a list of items to send.
>> +
>> +Each item can be a either a filename or plain text."
>
> I see that the way to distinguish between a filename and "plain 
> text" is
> `file-exists-p'.  Why not make filenames be a file: URL?

Ah sure. This is an option. file: didn't occur to me.

At a point, I was considering including type information per item 
like:

((type . file)
 (value . "/path/to/hello.txt"))

vs

((type . text)
 (value . "hello world"))

My gut told me to go with the simpler string option initially and 
consider including type information in a future iteration if users 
do have concrete needs for it.

> What about other URLs such as https:?  IMO, these should be 
> accounted
> for since it is quite common to send URLs (to other devices in 
> my case).

Yup. I do this frequently too. The current implementation (using 
file-exists-p) should handle sending URLs (ie. https://gnu.org) as 
text.

>> +  :type '(function :tag "Function")
>> +  :group 'send-to
>> +  :version "31.1")
>> +
>> +(defcustom send-to-context-items-function 
>> #'send-to--default-context-items
>> +  "A function to collect the items to be sent by `send-to'.
>> +
>> +Defaults to `send-to--default-context-items'.
>> +
>> +The function returns a list of items, where each item can be a 
>> either
>> +a filename or plain text."
>> +  :type '(function :tag "Function")
>> +  :group 'send-to
>> +  :version "31.1")
>
> We should document how a filename is distinguished from plain 
> text item
> here again.

Yep. Good point. Will do.

> Also should this be buffer-local so different major-mode
> can provide different providers?

Ah. Sure. Will do.

>> +         (error "Don't know how to sende %s (adjust 
>> `send-to-handler-function')"
>                                       ^^^^^ Typo here!
>

Oops. Thanks!

>> +  (cond ((derived-mode-p 'dired-mode)
>> +         (or
>> +          (send-to--dired-filenames-in-region)
>> +          (dired-get-marked-files)))
>> +        ((use-region-p)
>> +         (list (buffer-substring-no-properties
>> +                (region-beginning)
>> +                (region-end))))
>> +        ((buffer-file-name)
>> +         (list (buffer-file-name)))))
>
> Why not add (thing-at-point 'existing-filename)?

No reason other than I didn't know ;) I can use thing-at-point.