From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 6925C1FF0AA for ; Tue, 03 Nov 2026 15:40:31 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 59CB2218BC; Tue, 03 Nov 2026 15:40:27 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Tue, 03 Nov 2026 15:40:21 +0100 Message-Id: From: "Lukas Wagner" To: "Dominik Csapak" , "Lukas Wagner" , , Subject: Re: [PATCH proxmox-widget-toolkit v7 22/41] notifications: matcher: add better calendar editor X-Mailer: aerc 0.22.0-0-gc2f86b7abde3 References: <20261103104859.172466-1-l.wagner@proxmox.com> <20261103104859.172466-23-l.wagner@proxmox.com> <8f8b7a19-02f9-4505-bad2-271631301381@proxmox.com> In-Reply-To: <8f8b7a19-02f9-4505-bad2-271631301381@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1793716821510 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.217 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: ITIF3EJFQZ7OBNCROXNDAMSIAPVHRW3A X-Message-ID-Hash: ITIF3EJFQZ7OBNCROXNDAMSIAPVHRW3A X-MailFrom: l.wagner@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Tue Nov 3, 2026 at 2:56 PM CET, Dominik Csapak wrote: > a few high level ux things i noticed (some are pre-existing) > > same start/end time is valid (mon 12-12), not sure if that makes sense I chose to allow it because the DailyDuration type in rust also allows it, meaning that there could be existing configs (although not likely) that have this -- I wanted to be able to properly round-trip these. Semantically it of course does not make much sense to create such a calendar matcher, but it also does not really hurt. > ranges are only ever from mon to sun > e.g. checking sun,mon,tue could be sun..tue but instead it's > mon,tue,sun (no biggie) Fair! Good catch. There are other cases where non-optimal timerange strings are produced, e.g. when checking mon,tue,wed,fri,sat,sun -- it then enumerates the days instead of producing two ranges (mon..wed,fri..sun). Ultimately I think this is fine as is, maybe worth revisiting as a followup. The produced strings are 'correct' after all, just not optimal, if optimal is meaning 'shortest'. > > one issue though: the old input accepted times like: '0:1' e.g. > `mon 0:1-12:2` I'm cursing at how lenient the DailyDuration parser is, hehe. > > this will not be correctly parsed and hold up the window > though it is quetsionable if that should be accepted in the > first place, as it's a big ambiguous what is meant > > e.g. 12:1 -> 12:01 or 12:10? I just checked DailyDuration, 12:1 would be in fact 12:01.=20 Since the improved calendar editor now uses proper timefields for the time input, and no pre-defined time format seems to support this single-digit minute format, I'd be tempted to just leave it as is. I don't really want to add code to special-case this (hopefully) unlikely format. If we get any complaints in the forum/bugtracker, we can still improve this later. Existing matcher using this odd format will continue to work (since the format is accepted by the backend DailyDuration type), it's just that on the first edit of the particular rule in the tree, it would 'break'/marked invalid, requiring the user to re-enter the time/days. Does this seem reasonable? > > > code looks good to me > Thanks for the review!