Bug 445963 - weekday of the month
Summary: weekday of the month
Status: RESOLVED FIXED
Alias: None
Product: KOpeningHours
Classification: Applications
Component: normalization (show other bugs)
Version: unspecified
Platform: Other Other
: NOR normal
Target Milestone: ---
Assignee: David Faure
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-11-23 05:38 UTC by HubMiner
Modified: 2022-01-02 18:59 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description HubMiner 2021-11-23 05:38:52 UTC
Please add UTs:

IN:  Th[1-2] 09:30-11:45
OUT: Th[1-2] 09:30-11:45
// works today

IN:  Th[1,2] 09:30-11:45
OUT: Th[1,2] 09:30-11:45
// Today this needlessly changes to Th[1-2]

Justification: 
These are usually specified in complex rules and there is a good change that people that use this style are correct and we should avoid making cosmetic changes for those situations.
Comment 1 David Faure 2021-12-05 12:01:45 UTC
Yes, this would be nice. But it's a non-trivial undertaking, to replace a simple bitfield with a list of ranges. This needs Volker's grammar expertise :)
Comment 2 Volker Krause 2021-12-05 13:27:22 UTC
(In reply to David Faure from comment #1)
> Yes, this would be nice. But it's a non-trivial undertaking, to replace a
> simple bitfield with a list of ranges. This needs Volker's grammar expertise
> :)

The problem isn't really a grammar change here (that already models all this), but the structure representing this internally needs to be changed to hold the exact syntax tree. Ie. instead of a bit field this would probably need to be a linked list of number pairs.
Comment 3 HubMiner 2021-12-08 02:12:35 UTC
A related issue, let me know if you want a separate submission:
 IN: Mon[1,3,-1]
CURRENT: Mon[-1,1,3]
DESIRED: input unchanged

Why: negative numbers mean last or from the end.  Humanly speaking, last should be after first.
Comment 4 David Faure 2021-12-25 23:15:55 UTC
Volker: right, I didn't mean the grammar per se, but the "code in openinghoursparser.y" more generally. I'm no yacc expert, but I think I found what I had to do. Let me know in the change request what you think :)

HubMiner: this is indeed very related. With this change, the input sequence is now unchanged.
The question is whether [1,3,2] and [-1,2] should also remain unchanged (or whether they should be normalized to [1-3] and [2, -1]). For now this is simply all kept as is.
Comment 5 Bug Janitor Service 2021-12-25 23:16:20 UTC
A possibly relevant merge request was started @ https://invent.kde.org/libraries/kopeninghours/-/merge_requests/88
Comment 6 David Faure 2022-01-02 18:59:04 UTC
Git commit b4cbddb80275a34416b0f71bf6157fd48e1e8f70 by David Faure.
Committed on 02/01/2022 at 18:53.
Pushed by dfaure into branch 'release/21.12'.

Turn nthMask into a vector of entries, to preserve intervals and ordering

M  +6    -0    autotests/parsertest.cpp
D  +0    -69   src/lib/consecutiveaccumulator_p.h
M  +20   -17   src/lib/evaluator.cpp
M  +0    -1    src/lib/openinghours.cpp
M  +20   -14   src/lib/openinghoursparser.y
M  +29   -20   src/lib/selectors.cpp
M  +16   -1    src/lib/selectors_p.h

https://invent.kde.org/libraries/kopeninghours/commit/b4cbddb80275a34416b0f71bf6157fd48e1e8f70