link-hint.el icon indicating copy to clipboard operation
link-hint.el copied to clipboard

Remove :open-multiple, enable multiple/all by default

Open stepnem opened this issue 4 years ago • 3 comments

Maybe it was some kind of WIP, but in its current state requiring explicit configuration for multiple actions seems to make no sense; on the other hand I can't foresee any harm in just passing it through.

This also unbreaks multiple/all for :copy etc.

stepnem avatar Mar 27 '20 01:03 stepnem

I'd be okay with it being opt-out, but not all link types should work with the "multiple" actions. It doesn't make sense to be able to click multiple buttons that change the buffer in the current window, for example.

noctuid avatar Mar 31 '20 23:03 noctuid

Yeah, it was just an improvement (a crucial one for my usage, but of course that's subjective), not a perfect solution. Same goes for the other PRs: I was just trying to get the damn thing to work (after I installed it and found out it didn't). :-D

I tried to address your request briefly (see the new commit), but the more I try, the more problems with the current design I see. It's all intertwined together: at-point-p doubling as something returning the real thing to act upon, :describe trying to use the same thing for description, the whole link-hint--apply DWIM thing, and of course the problem discussed in #14.

Solving any of those issues requires a rewrite or at least significant refactoring of the whole, and I don't want to invest the energy into that (let alone the related discussions it would involve) right now.

So I'm just going to revert to the patched up version I have locally, which currently works well enough for my use, and maybe take it from there sometime in the future.

I still have some minor unrelated nits I can submit as PRs, but you can close all of the current ones.

Thank you and sorry for not being more helpful.

stepnem avatar Apr 01 '20 10:04 stepnem

I tried to address your request briefly (see the new commit)

The reason there is :<action>-multiple and not just :multiple is that the multiple commands may make sense for some actions for a type but not for others. An example: an action that opens something internally in the same buffer vs. in a web browser.

I was thinking of addressing this more simply. For example, link-hint-define-type could just merge a default settings list with the specified one (favoring the user-specified one), and the types that currently do not specify :open-multiple could specify :open-multiple nil (and :<action>-multiple nil for any actions that don't make sense).

Probably this simple way is not good enough. It would be annoying for user-created actions to have to manually add these keywords to existing types. It would probably be better to do something like have a whitelist, a blacklist, and defaults (e.g. being able to do something like :multiple '(copy) :multiple-default nil or :no-multiple '(browse-internal) :multiple-default t). I'd have to think about it more.

The reason current types don't have :copy-multiple is probably because I've never needed it, but it makes sense to have. For now, if you want copy-multiple, you could just iterate over the types and add :copy-multiple t to all of them. (Or just keep using your local version)

at-point-p doubling as something returning the real thing to act upon

Seems reasonable to do this since the code to check also returns the thing for some types.

:describe trying to use the same thing for description

Seems reasonable to use the textual thing as a default description.

the whole link-hint--apply DWIM thing

It's ugly, but it works.

Solving any of those issues requires a rewrite or at least significant refactoring of the whole, and I don't want to invest the energy into that (let alone the related discussions it would involve) right now.

I wouldn't write link-hint the same way now if I started from scratch. I've thought about completely rewriting it, and probably will eventually rewrite it on top of things.el (but things is nowhere close to ready).

That said, I don't see the current implementation as being fundamentally broken in terms of functionality. I'm not aware of any issues that would require a rewrite to fix.

Thank you and sorry for not being more helpful.

I apologize for any frustration caused by my package. My packages frustrate me as well, especially given my limited time to improve them.

noctuid avatar Apr 05 '20 15:04 noctuid