org-journal icon indicating copy to clipboard operation
org-journal copied to clipboard

Add two more insert at point commands

Open dalanicolai opened this issue 4 years ago • 11 comments

Inserting journal style headlines easily can be useful in non-journal buffers too. Especially when using any 'org-notifications' package (e.g. org-wild-notifier). This PR adds two more insert-at-point variants for the org-journal-new-entry and the org-journal-new-scheduled-entry commands.

dalanicolai avatar Feb 24 '21 15:02 dalanicolai

Should 412328e05d2c19339774db3b9e292d7f57db2c93 be part of this PR? It seems that the rest of the PR would work without it.

bastibe avatar Mar 06 '21 13:03 bastibe

I remember that I wrote somewhere that I kept the three commits separate, to make it more clear what happened in each commit as each commit adds a different feature (however I can not find back where I wrote that, so maybe I deleted that accidentally). But indeed each 'later' commit includes the changes in the earlier commits. So you could squash the commits, but I think the commits deserve to be separate.

dalanicolai avatar Mar 06 '21 21:03 dalanicolai

The other two commits seem fairly straightforward. But I don't quite understand the purpose or mechanism in 412328e05d2c19339774db3b9e292d7f57db2c93. It seems that it used to insert TODO, and now it will insert SCHEDULED. Is this intentional?

bastibe avatar Mar 09 '21 12:03 bastibe

Yeah sorry for the messy PR's. I thought I will purposely create multiple PR's so that you can pick the ones you like, while I can explain things in little chunks (well actually I don't remember exactly why I created multiple PR's, but I do remember that I purposely created multiple commits).

Anyway, to come back to your question... the PR does not remove the TODO, but instead it places it in front of the timestamp as is required for an org TODO (I wrote that in the message of PR #338, but I understand this message got lost in the mess).

Here are two screenshots after using the org-journal-new-scheduled-entry command before and after the commit: BEFORE Screenshot_20210309_163225

AFTER Screenshot_20210309_163510

Additionally, it adds the scheduled stamp, first because the command is called insert-new-SCHEDULED-entry, and second because the scheduled stamp enables to set custom alerts when using the org-wild-notifier package. (This requirement is only implicitly documented in its documentation, but I documented it explicitly for the Spacemacs org layer.

dalanicolai avatar Mar 09 '21 15:03 dalanicolai

Your example certainly makes sense. On my machine, org-journal-new-scheduled-entry does not put a time after the bullet. Is this a customization on your end?

bastibe avatar Mar 10 '21 13:03 bastibe

Do you mean before merging the PR or after merging the PR? (I assume you mean before merging the PR)... Well, looking at the source code, it sounds strange anyway. It looks like there should get a time inserted after the bullet by the org-journal--insert-entry function. It should also get inserted there when you just use org-journal-new-entry (which is also what org-journal-new-scheduled-entry uses). I don't see any reason why it would not get printed (unless you call it with a universal-argument).

I have tested it also on a fresh install, fresh Emacs with fresh org-journal, and then the time does get inserted when calling org-journal-new-scheduled-entry.

Using magit-blame I do find that Christian worked on that part on 26/01/2021. Are you sure you are using the latest version?

dalanicolai avatar Mar 10 '21 15:03 dalanicolai

Are you sure you are using the latest version?

I might not be. Good call.

bastibe avatar Mar 13 '21 09:03 bastibe

Sorry for the delay. Frankly, I feel unqualified for signing off this PR, as I don't understand it (haven't had time to dive deeply enough into it, and haven't been maintaining org-journal for a while).

Is this super important to you, or are you fine with waiting for @cslux to comment? I can probably take some time next week if it's important.

bastibe avatar Mar 21 '21 20:03 bastibe

No problem at all! I can also comment right at the chunks in the commits to help you figuring to save your time, but to make those commits relevant it would be nice if you could shortly hint on which aspect is not clear (the motivation or the implementation, or maybe it is a more specific thing. You could also add comments/questions right in the commits). Also, are you familiar with edebug? If not, it is really easy to start using it and can be really helpful for making sense what code is doing (just a recommendation). Finally, I wrote short motivations for each commit in their respective commit messages.

Also, if it needs more time to get merged it is no problem at all (just please indicate soon or not so soon :). I can simply make Spacemacs use my fork for the time being. Anyway, thanks for looking after it.

dalanicolai avatar Mar 21 '21 21:03 dalanicolai

I've had a talk with @cslux, and he said he'd be back in a couple of weeks. I'll defer this PR until then if that's OK with you.

bastibe avatar Apr 21 '21 06:04 bastibe

No problem! Thanks for the update... in the meantime I will just point Spacemacs to my fork.

dalanicolai avatar Apr 21 '21 08:04 dalanicolai