Morgan/kill but reversable #2934
Conversation
There was a problem hiding this comment.
This is great, thanks!
I replied to you on Matrix about the killStack but I'll reproduce it here:
> <@mci:matrix.org> Hello! I'm I'd like to work on this issue https://github.com/tridactyl/tridactyl/issues/2794 . Is content/state_content.ts the place to store the "killStack" ?
Hi! Thanks for that PR. It looks good.
I wouldn't personally have put it there - we use it for stuff that needs to be shared between multiple scripts and particularly for state that other things need to be told about when there is a change. I would just do
"""
//#content_helper
const KILL_STACK = Element []
"""
above the hint function.
It doesn't really matter though : )
src/excmds.ts
Outdated
| @@ -4224,13 +4224,17 @@ export async function hint(option?: string, selectors?: string, ...rest: string[ | |||
| selectHints = hinting.pipe_elements( | |||
| hinting.killables(), | |||
| elem => { | |||
| elem.remove() | |||
There was a problem hiding this comment.
I think we should probably leave this in place and make a new "mode" for reversible deaths. The .remove part I think stops JavaScript from messing with an element so it probably would still be useful on particularly unruly sites.
| return elem | ||
| }, | ||
| rapid, | ||
| ) | ||
| break | ||
|
|
||
| case "-K": |
There was a problem hiding this comment.
I'm slightly uneasy about this being a hint mode. It sort of makes sense as it is related to the code above ... but it also doesn't as there are no hints to select. I think on balance it's probably OK.
src/static/clippy/3-hint_mode.md
Outdated
| @@ -9,6 +9,7 @@ Here are some of the most useful hint modes: | |||
| - `:hint -p` or `;p`: copy element text (such as a paragraph) to clipboard | |||
| - `:hint -#` or `;#`: copy anchor location. Useful for linking someone to a specific part of a page. | |||
| - `:hint -k` or `;k`: kill an element. Very satisfying. | |||
| - `:hint -K` or `;K`: restore killed elements in LIFO order. Fixes mistakes. | |||
There was a problem hiding this comment.
| - `:hint -K` or `;K`: restore killed elements in LIFO order. Fixes mistakes. | |
| - `:hint -K` or `;K`: restore elements most recently killed via `:hint -k`. Fixes mistakes. |
I think LIFO might not be understood by all our users. I also want to make sure it's clear that it they manually kill an element some other way this won't magically restore it : )
…s killed elements
|
@bovine3dom Thank you for the feedback! I'm much more fond of this PR now. I was able to implement all of your comments, but let me know if you see any other possible improvements. In particular, let me know if you think there are better names for the new reversible kill hint mode, now |
src/excmds.ts
Outdated
| - `bind ;c hint -c [class*="expand"],[class="togg"]` works particularly well on reddit and HN | ||
| - this works with most other hint modes, with the caveat that if other hint mode takes arguments your selector must contain no spaces, i.e. `hint -c[yourOtherFlag] [selector] [your other flag's arguments, which may contain spaces]` | ||
| - -f [text] hint links and inputs that display the given text | ||
| - -p copy an element's text to the clipboard -h select an element (as if you click-n-dragged over it) -P copy an element's title/alt text to the clipboard -r read an element's text with text-to-speech -i view an image -I view an image in a new tab -k delete an element from the page -K deletes an element from the page; deleted elements can be restored using :restorekilled. -s save (download) the linked resource -S save the linked image -a save-as the linked resource -A save-as the linked image -; focus an element and set it as the element or the child of the element to scroll -# yank an element's anchor URL to clipboard -c [selector] hint links that match the css selector `bind ;c hint -c [class*="expand"],[class="togg"]` works particularly well on reddit and HN this works with most other hint modes, with the caveat that if other hint mode takes arguments your selector must contain no spaces, i.e. `hint -c[yourOtherFlag] [selector] [your other flag's arguments, which may contain spaces]` -f [text] hint links and inputs that display the given text |
There was a problem hiding this comment.
it looks like some formatting was lost here
src/content/state_content.ts
Outdated
| @@ -20,13 +20,15 @@ export class PrevInput { | |||
| class ContentState { | |||
| mode: ModeName = "normal" | |||
| suffix = "" | |||
| killStack: Element[] = [] | |||
There was a problem hiding this comment.
I don't think we need any of the changes to this file any more
src/excmds.ts
Outdated
| *NB: The most recently killed element that has not already been restored will be restored. | ||
| */ | ||
| //#content | ||
| export async function restorekill() { |
There was a problem hiding this comment.
Yeah, I don't know what to call this either. In general we have a convention of naming things [object][action], so under that this would be killrestore ... but I don't like that either : )
I'll try to think about it a bit more.
There was a problem hiding this comment.
How about resurrect(), revive() or elementrestore()?
There was a problem hiding this comment.
I like elementrestore the most out of those ... still not 100% happy with it though.
I'll try really hard to get round to look seriously at this PR next week : )
bovine3dom
force-pushed
the
morgan/kill-but-reversable
branch
from
8f39d12
to
abdd160
Compare
|
LGTM - I'll merge once CI has passed. Thanks, and sorry it took so long to review :) |
adds hintmode excmd
-Kwhich restores killed elements in LIFO.-Kfor unKillfixes #2794