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

Skip marking calendar entries when calendar is just for input #375 #379

Closed dppdppd closed 2 years ago

dppdppd commented 2 years ago

The more I thought about the previous PR, the less I liked it.

For one, it had the issues you raised, which is that it's not clear what calendar integration means wrt bindings. Should mark-entries still be mapped so users can manually mark them? And if they do, should the previously disabled commands be now enabled? And if none of the bindings are disabled, some of them would lead to confusion if the calendar isn't marked.

And secondly, regardless of what one does with the bindings, it didn't prevent the problem. An affected user would stall on calendar popups without any explanation or hint as to what package was doing it or how to fix it.

What do you think about this alternative? The calendar is not marked with journal entries when the calendar is popped up as a result of org-read-date. In my limited usage, journal entry dates aren't useful when picking a date for timestamp entry. But maybe they are in some contexts? More importantly, this eliminates the need to debug and identify the solution in the first place.

dppdppd commented 2 years ago

Of course.

We only mark entries when org-journal-dont-mark-entries is nil. In the case of the org-read-date (which calls calendar), we toggle it on beforehand and back off right after.

So if the user calls (calendar) then they see journal markings. And if they call, say, (org-time-stamp), then they don't.

I did not make it customizable but can.

bastibe commented 2 years ago

But why do we need to set org-journal-dont-mark-entries before calling org-read-date? I thought org-journal-mark-entries doesn't do anything anyway if org-journal-dont-mark-entries is t.

Also, I think a positive variable name is better than a negative one, so org-journal-should-mark-calendar instead of org-journal-dont-mark-entries. And the variable name should clarify that we're talking about calendar entries.