Tags plugin - Githubissues

platers commented 3 years ago

Adds tagging #249.

Very similar to marks, but non unique. Each tag has a tag root, which stores clones of all rows with that tag. Each tag root is marked for easy lookup.

'#' to create a tag. 'd#[number]' to delete a tag with index [number] (1 indexed) 'd#' to delete a tag if the row only has one tag

Cloning logic: The tag root is the node with mark = tag Rows are cloned when a tag is added to that row. Rows are uncloned when a tag is deleted from that row. If the tag root does not exist, one is generated at the document root and all tagged rows are cloned. I'm not sure if this logic is best.

Should probably add unit tests

WuTheFWasThat commented 3 years ago

whoah, that's cool! I will try/look at it sometime soon. I didn't understand your notes about cloning logic though, could you explain?

platers commented 3 years ago

the cloning logic is how the clones are maintained. Currently the clones are only updated when a tag is added or deleted from a row, or when the tag root is deleted. This allows the user to mess around in the tag root, and the cloned rows might get out of sync with the actual rows with that tag. The only way to refresh the tag root is to delete it then tag a new row. Also in this scheme, rows are not deleted from the tag root even if that row no longer exists elsewhere in the document.

I could imagine another scheme where the tag roots are refreshed all the time to make sure they are always in sync. Then the user wouldnt be able to change much in the tag root.

WuTheFWasThat commented 3 years ago

I see, if the tags are non-unique then I don't think they should share cloned children. if you want that functionality, you should instead just clone a row with marks and have them as children, i think?

platers commented 3 years ago

hmm I'm not sure what you mean. I think the tags should share cloned children. If I tag a row #todo and #urgent I want that row to show up in both the #todo and #urgent roots.

WuTheFWasThat commented 3 years ago

ahh, i see. interesting! it's a cool feature, not what i had in mind

your logic soudns good. what happens if you add a child to the tag root? (seems fine if nothing happens)

platers commented 3 years ago

yep nothing happens. My goal was to generalize #300. Now that I think about it, a tag search where you search for a tag and it returns the rows might be useful to for easier navigation. Not sure if that should be an additional feature or if it should replace cloning.

WuTheFWasThat commented 3 years ago

yeah, the version of tags i had imagined in the past was:

you can tag rows with any number of tags
you can quickly search for everything with a given tag

however, your version is nice since searching isn't as good as seeing them all in one place

perhaps the basic version of tags can be a core feature of vimflowy, and then the extra feature of adding marks for each unique tag with cloned children could be a plugin (called "Tags To Mark" or something)

what do you think?

platers commented 3 years ago

that sounds good to me! I'll work on that.

platers commented 3 years ago

should be ready to try now. Search tags with '-'. Tag cloning is now another plugin.

WuTheFWasThat commented 3 years ago

awesome. will take a look over the weekend

platers commented 3 years ago

addressed all your comments, let me know if you want more tests. Also I added a test-windows command in package.json to run unit tests on windows.

WuTheFWasThat commented 3 years ago

tested briefly.

undo of deleting tags doesn't seem to work (probably good to add a test for that too :))
it might be nice if they looked different from marks, maybe they can be square without the rounded edges?
it could be nice to add some stuff to the default document, at src/assets/ts/configurations/vim.ts
nit: not a big deal if it's hard for you to get lint working, but there are many lint errors in clone_tags/index.tsx, mostly variables that were defined but never used

platers commented 3 years ago

made tags square
added description to the default doc
for some reason lint isn't running on the clone tags plugin, I deleted as much as I can by eye
fixed undo for deleting tags. I had a bug where repeating d#1 with . froze the doc since the last tag would be deleted with just d#, leaving the 1 hanging. To fix this I removed the d# shortcut. Added tests for undo/redo/repeat.

found some bugs in tag clone, working on fixes

WuTheFWasThat commented 3 years ago

nice!

that's too bad about the deleting tags... why does the hanging 1 freeze the doc? (most likely my fault, i just don't remember the code)

when i delete a tag then undo, it goes to a new position. however, redo still works - i guess because the mutation stored is by tag name and not index. this seems fine to me, seems a bit tricky overall and scary since undo doesn't quite restore the document to exactly the same state... not necessarily problematic right now though but it would be if there was also something that relied on tag indices

here's what i see for linting:

platers commented 3 years ago

made tags sorted
lots of bug fixes in tag clone
fix weird repeat behavior in tags Turns out the repeat freeze was because it was waiting to read a character forever. I fixed that, but couldn't figure out a nice way to implement the d# shortcut.

WuTheFWasThat commented 3 years ago

hm, so what happens if in the d# shortcut you just don't wait for a number if there's exactly one tag?

platers commented 3 years ago

it made it freeze after repeating d#1 with '.' I'm not sure exactly why

WuTheFWasThat commented 3 years ago

ok I'll try it real quick

WuTheFWasThat commented 3 years ago

hmm it's a tricky bug in my code...

WuTheFWasThat commented 3 years ago

i pushed a fix to master, so something like this should no longer cause hangs

platers commented 3 years ago

nice! The shortcut seems to work now but some of the unit tests now fail. The 'can repeat' test gives 'Error: Queue stopped: queue is stopped!!'. Is there a difference between real use and sendKeys?

WuTheFWasThat commented 3 years ago

it seems to pass for me, could you push the latest tests to your branch?l

platers commented 3 years ago

I just added the shortcut, no changes to the tests.

WuTheFWasThat commented 3 years ago

on line 66, this needs to change:

t.sendKeys('kd#1');

to

t.sendKeys('kd#');

otherwise it's waiting for something after the 1

platers commented 3 years ago

oh that makes sense. 'can repeat' seems to have a different issue. d#1. works when I try it manually, but the test case is giving the queue is stopped error.

WuTheFWasThat commented 3 years ago

you were right, there was a weird issue affecting unit tests but not the browser. it's fixed now on master. in addition to that, i had these changes, after which tests pass! https://github.com/WuTheFWasThat/vimflowy/commit/8f073e6b2c65d03b9d7083e7c3c1482b41b35a32

WuTheFWasThat commented 3 years ago

merged

platers commented 3 years ago

woot! I'm still working on some bugs in clone tags (ctrl+r doesnt work) but the core tags plugin should be completely functional.

WuTheFWasThat commented 3 years ago

ah ok sounds good

WuTheFWasThat / vimflowy

Tags plugin #364