Can someone review? @schoettl @munen

@schoettl Did you use nvm?

@schoettl Did you use nvm?

I tried but couldn't even install it on NixOS :/ I'll try again later this week.

Hi @vHugoObject, I got my testing environment right and started playing. I like this feature, good work!

For reference:

• https://orgmode.org/guide/Progress-Logging.html
• https://orgmode.org/manual/Closing-items.html

I found a few bugs that you might want to fix:

1. The CLOSED: […] word should be in the same line as SCHEDULED: or DEADLINE:
2. The CLOSED: should be removed when the header's TODO state is changed to something other than a DONE state.
3. If CLOSED: exists, another CLOSED: is added when I close a header again. The timestamp should be updated instead.
4. The CLOSED: is currently placed below a :PROPERTIES: drawer (see screenshot below). It should be above it, right below the headline.
5. The settings either :LOGGING: in a drawer or #+STARTUP: should be case-insensitive. E.g. :logging: logdone should have the same effect.

I recommend that you look into how SCHEDULED and DEADLINE are implemented, it should be pretty similar regarding parsing, output, and storing.

Hi @schoettl! I recently re-did my entire Org setup and have moved from Orgzly to Organice so that I can also have my team mates use Org mode to track a project we started working on.

I noticed this feature was missing and came across the original issue thread and this PR. I would dearly love for this feature to exist, so that it would match similar behaviour on my Org mode setup on Emacs.

If it's okay with @vHugoObject and they have not worked on the fixes yet, I would like to pick up the fixes you have mentioned above. I have some time towards the end of this week that I can dedicate to these fixes if so.

Hi @aharish, that would be very nice if you could pick up and finish this!

Maybe it's a good idea to merge master into this PR before you continue.

Do you plan to use Organice on the phone or desktop? On the phone, Organice currently might feel fragile because with vertical scroll it's too easy to accidentally swipe horizontally and delete items or set them to DONE. See #995 – just as a warning.

Hi @aharish, that would be very nice if you could pick up and finish this!

Great! Will work on this then.

Maybe it's a good idea to merge master into this PR before you continue.

Yes, will do.

Do you plan to use Organice on the phone or desktop? On the phone, Organice currently might feel fragile because with vertical scroll it's too easy to accidentally swipe horizontally and delete items or set them to DONE. See #995 – just as a warning.

Both phone and desktop. At least I will be using it on the phone, my team mates probably not as much. I did notice that issue. I have run into a few other bugs/QOL improvements that can be made for a much better UX overall. I will compile a list and post them.

Hi @schoettl - I just wanted to drop an update here as I have been working on this, and also clarify a couple of questions I had:

• I have reimplemented the logic for adding the CLOSED: [...] entry using the same logic that is used in SCHEDULED: [...] or in DEADLINE: [...]. This means we can reuse a lot of the same functions for the planning items instead of creating new ones for the CLOSED entry. So updating the CLOSED entry or removing it when the TODO state changes becomes very trivial to implement.

• I tested out the functionality from this PR, and I noticed that the CLOSED: [...] entry only fires when swiping the item, or toggling it through clicking on the TODO state in the header. It doesn't fire when the one of the TODO states are clicked when editing the title. I would like to add support for this as well in order to not have a broken UX.

• Another thing that I noticed was that the header is only created when the last DONE state is activiated. So let's say that you have setup #+TODO_SEQ: as below

  #+TODO_SEQ: TODO NEXT WAITING | DONE CANCELLED
  

  The CLOSED entry is only added when the header is marked as CANCELLED, which might not be the most ideal case scenario since both DONE and CANCELLED are valid DONE states.

  I want to implement this the right way, and wanted to get your inputs on this.

• I also wanted your input on one other thing as well - currently with my implementation, the CLOSED header gets added to the end of the planning items line. I think on Emacs, the official implementation acutally added this at the start of the line. Should we follow the same format so as to keep consistency with the way Org mode works?

Hi @aharish, nice to hear you had time to work on this!

Hi @schoettl - I just wanted to drop an update here as I have been working on this, and also clarify a couple of questions I had:

• I have reimplemented the logic for adding the CLOSED: [...] entry using the same logic that is used in SCHEDULED: [...] or in DEADLINE: [...]. This means we can reuse a lot of the same functions for the planning items instead of creating new ones for the CLOSED entry. So updating the CLOSED entry or removing it when the TODO state changes becomes very trivial to implement.

Sounds great!

• I tested out the functionality from this PR, and I noticed that the CLOSED: [...] entry only fires when swiping the item, or toggling it through clicking on the TODO state in the header. It doesn't fire when the one of the TODO states are clicked when editing the title. I would like to add support for this as well in order to not have a broken UX.

Would be cool if you could fix such UX bugs on the way!

• Another thing that I noticed was that the header is only created when the last DONE state is activiated. So let's say that you have setup #+TODO_SEQ: as below

  #+TODO_SEQ: TODO NEXT WAITING | DONE CANCELLED
  

  The CLOSED entry is only added when the header is marked as CANCELLED, which might not be the most ideal case scenario since both DONE and CANCELLED are valid DONE states.
  I want to implement this the right way, and wanted to get your inputs on this.

I would also consider that as a bug though I don't know for 100% how it behaves in Emacs.

• I also wanted your input on one other thing as well - currently with my implementation, the CLOSED header gets added to the end of the planning items line. I think on Emacs, the official implementation acutally added this at the start of the line. Should we follow the same format so as to keep consistency with the way Org mode works?

So that's not a big deal in my opinion but behaving just like Emacs would be best.

I just want to point to some bugs I found earlier, but maybe you already saw it: #984 (comment)

I'm happy to review your changes in this or a new PR once you're ready to push :)
Just as a note, I'm on holiday for the weekend so I can't respond before Monday.