search

bastibe / org-journal

A simple org-mode based journaling mode
BSD 3-Clause "New" or "Revised" License
1.24k stars 123 forks source link

Add two more insert at point commands #339

Open dalanicolai opened 3 years ago

dalanicolai commented 3 years ago

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.

bastibe commented 3 years ago

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

dalanicolai commented 3 years ago

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.

bastibe commented 3 years ago

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?

dalanicolai commented 3 years ago

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.

bastibe commented 3 years ago

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?

dalanicolai commented 3 years ago

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?

bastibe commented 3 years ago

Are you sure you are using the latest version?

I might not be. Good call.

bastibe commented 3 years ago

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.

dalanicolai commented 3 years ago

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.

bastibe commented 3 years ago

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.

dalanicolai commented 3 years ago

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