Add support for Markdown-style tasks in lists

agateau / nanonote

A minimalist note taking application

Other

59 stars 11 forks source link

Add support for Markdown-style tasks in lists #30

Closed dlaidig closed 1 year ago

dlaidig commented 1 year ago

The syntax for tasks is - [ ] or * [ ] for unchecked tasks and - [x] or * [x] for checked tasks. Tasks are highlighted and treated like links and the completion can be toggled with Ctrl+Enter, Ctrl+Click, and via the context menu. There is another keyboard shortcut that turns the current line into a task and, if it already is a task, toggles it. This way, it is not necessary to manually type the brackets.

I've been using this for the last few days and found it quite useful. I'm curious what you think and if you even want this feature in nanonote. (At least when not using the feature, the impact is minimal. Just one more entry in the context menu.)

Some other notes:

I expect that you will want to see some tests before merging this. I'll work on that if your general feedback is positive.
I also appreciate any other feedback about the code or about how this feature should behave. (At least, how I handled the "Toggle task completion" action in the context menu does not seem ideal.)
The implementation is not as self-contained as I would have liked. There can only be one syntax highlighter, so I had to extend the existing one. For auto-inserting the task check boxes when pressing enter, it is a lot easier to add this to the IndentExtension than to duplicate most of the functionality.
As the keyboard shortcut for the "Insert/toggle task" action, I now use Alt+T because Ctrl+T is used for "Always on Top". The obvious alternative would be Ctrl+Shift+T but that is harder to type. Personally, I quite like the idea of using Ctrl+T for the task action and Ctrl+Shift+T for "Always on Top". "Always on Top" is really useful sometimes, but over the years, I probably enabled it more often by accident than on purpose. Making this a bit harder is actually an improvement for me. Still, you probably don't want to change the shortcuts you chose, and I'm also happy to just patch the shortcuts for myself.

agateau commented 1 year ago

First of, I love the idea! It's a feature I have been thinking about for time to time, so it's great to see it happening!

Regarding the feature itself:

Ctrl+Enter seems to only work when in the checkbox, whereas Alt+T works for the whole line maybe we can remove the Ctrl+Enter shortcut?
The highlighted space in the checkbox looks a bit odd:

Maybe the whole checkbox should be tinted. I am not sure yet, but this is something we can iterate on later.

Regarding Alt+T vs Ctrl+T, what do you think of using Ctrl+Space?

PS: following my CI changes, you have to rebase your branch on master, for the CI to be happy.

dlaidig commented 1 year ago

First of, I love the idea! It's a feature I have been thinking about for time to time, so it's great to see it happening!

Thanks! I'm glad that you also find this useful. :)

Regarding the feature itself:

* Ctrl+Enter seems to only work when in the checkbox, whereas Alt+T works for the whole line maybe we can remove the Ctrl+Enter shortcut?

Just to explain the background: At first, I thought it is enough to detect the boxes and treat them like links, i.e., highlight them and toggle the box when activating the link. This also makes it easy to discover for someone who uses the Markdown task syntax. Ctrl+Enter and Ctrl+Click come from this first iteration.

Then I added Alt+T as an "Insert task" shortcut because typing the square brackets is annoying. If there already was a task, this action would not do anything.

Then I realized that toggling tasks from anywhere in the line would be really useful, so I extended the functionality.

The reason I thought that Ctrl+Enter should still exist is for consistency with opening regular links.

I have also thought about completely removing the highlighting and the link-like functionality. The only drawback is that it reduces the discoverability and that tasks cannot be toggled with the mouse any more (Ctrl+Click).

* The highlighted space in the checkbox looks a bit odd:
Maybe the whole checkbox should be tinted. I am not sure yet, but this is something we can iterate on later.

I agree, although I got used to it. In the first iteration, I highlighted the whole 3 characters. I can tell you that underlined square brackets do not look good...

An idea that might be worth testing is using a blue-but-not-underlined style for [ ]/[x]. This way, it will look different than regular links but probably still close enough.

Regarding Alt+T vs Ctrl+T, what do you think of using Ctrl+Space?

In general, I'm not against it, but on macOS this is the default shortcut for the Spotlight search, so I doubt it will work there. Another idea: Ctrl+Enter?

(Ctrl+Enter and Ctrl+Space seem quite natural for toggling tasks, but might be a bit unusual for inserting tasks.)

PS: following my CI changes, you have to rebase your branch on master, for the CI to be happy.

dlaidig commented 1 year ago

In general, I'm not against it, but on macOS this is the default shortcut for the Spotlight search, so I doubt it will work there. Another idea: Ctrl+Enter?

Oh, and of course, when wrote this, I didn't realize that Ctrl+Enter is the shortcut to open links. It could be still be used for this:

If the cursor is inside a link, open the link.
Else, if the cursor is inside a line with a task, toggle it.
Else, turn the current line into a task.

I'm not sure if this is really a good idea, though.

agateau commented 1 year ago

An idea that might be worth testing is using a blue-but-not-underlined style for [ ]/[x]. This way, it will look different than regular links but probably still close enough.

Sounds like a good idea, indeed.

In general, I'm not against it, but on macOS this is the default shortcut for the Spotlight search, so I doubt it will work there. Another idea: Ctrl+Enter?

If Ctrl+Space is not available on macOS, let's not use it. Ctrl+Enter is undocumented right now: the "Open link" menu entry does not mention the shortcut (to the point that I forgot it existed...). I suggest the following:

Change "Open link" to "Go to link" with the Ctrl+G shortcut (for Go)
Use Ctrl+Enter to add/toggle checkboxes

Does it sound good to you?

dlaidig commented 1 year ago

An idea that might be worth testing is using a blue-but-not-underlined style for [ ]/[x]. This way, it will look different than regular links but probably still close enough.

Sounds like a good idea, indeed.

I have changed the style like this and I quite like it.

(I think it might even be worth to consider removing the underline for regular links. Personally, I'm quite undecided about what I like better. In any case, it's your decision and out of scope for this pull request.)

* Change "Open link" to "Go to link" with the Ctrl+G shortcut (for Go)

* Use Ctrl+Enter to add/toggle checkboxes

Does it sound good to you?

I do like this idea and think it is worth trying out.

It is not as trivial as changing the shortcut definition since the current "Open link" shortcut is not a QAction but instead handled in the keyPress event. To make it more complicated, I tried to treat tasks like links, and therefore this event was handled in two extensions.

The main question now is if Ctrl+G should also toggle tasks if the cursor is inside the task checkbox. The reasoning for this is that if it looks like a link, it should be possible to activate it like a link (especially since Ctrl+Click already works). On the other hand, one could argue that the toggle shortcut Ctrl+Enter has the same effect and it is confusing if there are two shortcuts that do the same in this situation.

In the second case, it should be rather straightforward to turn "Go to link" into a QAction within the LinkExtension. In the first case, though, it is not that clear how to handle this shortcut to work on both links and tasks.

agateau commented 1 year ago

An idea that might be worth testing is using a blue-but-not-underlined style for [ ]/[x]. This way, it will look different than regular links but probably still close enough.

Sounds like a good idea, indeed.

I have changed the style like this and I quite like it.

Just tried it, I think it's nice but maybe a bit too subtle now. I am experimenting with a slight background color, but I am not there yet.

(I think it might even be worth to consider removing the underline for regular links. Personally, I'm quite undecided about what I like better. In any case, it's your decision and out of scope for this pull request.)

I prefer keeping the underline, it is more self-explanatory this way.

It is not as trivial as changing the shortcut definition since the current "Open link" shortcut is not a QAction but instead handled in the keyPress event. To make it more complicated, I tried to treat tasks like links, and therefore this event was handled in two extensions.

It would probably be cleaner to turn the "Open link" shortcut into a QAction. I can look into this and make a separate PR for this if you don't feel like tackling it.

The main question now is if Ctrl+G should also toggle tasks if the cursor is inside the task checkbox. The reasoning for this is that if it looks like a link, it should be possible to activate it like a link (especially since Ctrl+Click already works). On the other hand, one could argue that the toggle shortcut Ctrl+Enter has the same effect and it is confusing if there are two shortcuts that do the same in this situation.

I don't think Ctrl+G should toggle tasks. They are two different actions in my opinion, and it's going to be confusing if a task item also contains a link.

dlaidig commented 1 year ago

I have changed the style like this and I quite like it.

Just tried it, I think it's nice but maybe a bit too subtle now. I am experimenting with a slight background color, but I am not there yet.

I find it to be clear enough to recognize, and I would be worried that too much highlighting might actually distract from the content. I guess it also depends quite a bit on the theme, and in dark themes the blue color stands out more:

plasma_dark

plasma_light2

Anyway, feel free to change the style however you like. :)

It is not as trivial as changing the shortcut definition since the current "Open link" shortcut is not a QAction but instead handled in the keyPress event. To make it more complicated, I tried to treat tasks like links, and therefore this event was handled in two extensions.

It would probably be cleaner to turn the "Open link" shortcut into a QAction. I can look into this and make a separate PR for this if you don't feel like tackling it.

The main question now is if Ctrl+G should also toggle tasks if the cursor is inside the task checkbox. The reasoning for this is that if it looks like a link, it should be possible to activate it like a link (especially since Ctrl+Click already works). On the other hand, one could argue that the toggle shortcut Ctrl+Enter has the same effect and it is confusing if there are two shortcuts that do the same in this situation.

I don't think Ctrl+G should toggle tasks. They are two different actions in my opinion, and it's going to be confusing if a task item also contains a link.

In my suggestion, Ctrl+G would only have worked when the cursor is inside the [ ], just to stay compatible with Ctrl+Click (which I really would like to keep for toggling tasks via the mouse). But I'm happy that you don't think this is important since that makes the implementation a lot simpler.

I'll attempt to change it like you suggested and finally add unit tests. (Unfortunately, it might take some time.)

dlaidig commented 1 year ago

I've now changed the shortcuts to Ctrl+G/Ctrl+Enter. That did simplify the code quite a bit. :) (Tests are still missing.)

agateau commented 1 year ago

New version looks promising! Let me know if you need help with the tests.

dlaidig commented 1 year ago

I added some tests. It's definitely not exhaustive, but that also applies to the existing tests. (I noticed that there was nothing to test the auto-completion for new list items, so I added one.)

Let me know if there are some specific tests that I should still add.

(And I still noticed some minor jumping cursor annoyance when undoing the "insert task" action, which is now fixed.)