Skip to content

fix(agenda): align multi-week spans to the start weekday - #1178

Open
liamrlawrence wants to merge 1 commit into
nvim-orgmode:masterfrom
liamrlawrence:fix/org_agenda_start_on_weekday
Open

fix(agenda): align multi-week spans to the start weekday#1178
liamrlawrence wants to merge 1 commit into
nvim-orgmode:masterfrom
liamrlawrence:fix/org_agenda_start_on_weekday

Conversation

@liamrlawrence

Copy link
Copy Markdown
Contributor

Summary

org_agenda_start_on_weekday was only honored for the week span and the
string '7', so numeric spans never aligned. Setting org_agenda_span = 14
started the range on today rather than the configured weekday, leaving no way
to get a two-week agenda that begins on a week boundary.

The documentation disagreed with itself and with the code:

Source Claim
docs/configuration.org applies to week and number span
lua/orgmode/config/_meta.lua:207 applies only to week span
_set_date_range span == 'week' or span == '7'

Changes

Resolve the span to a day count and align whenever it covers whole weeks
(7, 14, 21, ...). Spans with no fixed length (day, month, year) and
partial weeks such as 10 keep starting from today.

Partial weeks are excluded deliberately: advance_span steps by exactly one
span, so only whole-week jumps preserve the alignment when paging with
f/b. Aligning a 10-day span would give one correct view and then drift,
which is worse than never aligning.

On the removal of the '7' special case

The removed span == '7' branch was not working behavior thus was dropped. A
string span aligns correctly, but then crashes immediately afterward in the
same function:

    local modifier = { [span] = 1 }
    if type(span) == 'number' then
      modifier = { day = span }
    end
    to = from:add(modifier)

With span = '7', type(span) == 'number' is false, so modifier becomes
{ ['7'] = 1 }, and Date:add does date[opt] = date[opt] + val against an
osdate field that doesn't exist, which throws the error: attempt to perform arithmetic on a nil value.

It is also unreachable through config, since Config:get_agenda_span
validates string spans against {'day', 'month', 'week', 'year'} and falls
back to week. Only an explicit org_agenda_span = '7' in a custom command's
opts gets past that, straight into the crash. tonumber replaces the branch
with the numeric spans that actually work.

Numeric-string spans remain broken elsewhere (advance_span builds
{ [self.span] = direction } the same way, _get_title would print
'7'-agenda). Fixing that properly means normalizing span to a number at
construction and is left out of scope.

Relation to Emacs

Emacs restricts this to 7 and 14 days. From the org-agenda-start-on-weekday
docstring in lisp/org/org-agenda.el:

(defcustom org-agenda-start-on-weekday 1
  "Non-nil means start the overview always on the specified weekday.
0 denotes Sunday, 1 denotes Monday, etc.
When nil, always start on the current day.
Custom commands can set this variable in the options section.

This variable only applies when agenda spans either 7 or 14 days."
  :group 'org-agenda-daily/weekly
  :type '(choice (const :tag "Today" nil)
		 (integer :tag "Weekday No.")))

...

	   (org-agenda-start-on-weekday
	    (and (or (eq ndays 7) (eq ndays 14))

Note the check is on ndays, so the integer 14 does align in Emacs
(org-agenda-ndays-to-span normalizes it to fortnight first).

This means that accepting any multiple of 7 is a deliberate superset.
I'm happy to narrow it to days == 7 or days == 14 for exact parity if
you'd prefer, but I think aligning with multiples of 7 makes more sense than
some arbitrary limit. I can easily imagine myself using a 21 or 28 day agenda.

Type hints

OrgAgendaTypeOpts.start_on_weekday and OrgAgendaType.start_on_weekday were
annotated number, but false is supported: the constructor uses
utils.if_nil rather than or specifically so an explicit false isn't
replaced by the config default, and if self.start_on_weekday then consumes it.

_meta.lua already typed the user-facing option as number | false, and
configuration.org documented the behavior ("If set to false, starts from today")
while its Type: line said number. Corrected everywhere to match, which is why
agenda.lua has changes outside _set_date_range.

Tests

Added three tests to verify behavior.

Checklist

I confirm that I have:

  • Followed the
    Conventional Commits
    specification
    (e.g., feat: add new feature, fix: correct bug,
    docs: update documentation).
  • My PR title also follows the conventional commits specification.
  • Updated relevant documentation, if necessary.
  • Thoroughly tested my changes.
  • Added tests (if applicable) and verified existing tests pass with
    make test.
  • Checked for breaking changes and documented them, if any.

org_agenda_start_on_weekday was only honored for the `week` span and
the string '7', so numeric spans never aligned. Setting
org_agenda_span = 14 started the range on today rather than the
configured weekday.

Resolve the span to a day count and align whenever it covers whole
weeks (7, 14, 21, ...). Spans with no fixed length and partial weeks
keep starting from today: advance_span steps by exactly one span, so
only whole-week strides preserve the alignment when paging.

Also corrects the start_on_weekday annotations to `number | false`,
which the constructor has always supported via utils.if_nil.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant