Add --every / -e option to the new command #1052

ghost · 2021-05-26T08:48:30Z

This includes support for interactive setting or editing an event. Fixes #999

This includes support for interactive setting or editing an event.

doc/source/usage.rst

khal/controllers.py

Update IRC references to Libera.Chat

Typo Co-authored-by: Hugo Barrera <[email protected]>

Fix #999. This also addresses a defect where editing a previously saved event cause the 3 rrule properties (freq, until and interval) to have incorrect default type (list instead of str), and freq value is forced to lower case as the current code requires it but more important that is how the values are presented in the interactive session.

khal/controllers.py

WhyNotHugo · 2021-05-26T22:51:57Z

Looks good! Just a tiny typo.

Typo. Co-authored-by: Hugo Barrera <[email protected]>

d7415

There are a few things here. It could probably also do with a test.

I'm open to opinions, but I also think we should probably add this to ikhal at the same time to prevent events created in khal that can't be fully edited in ikhal.

d7415 · 2021-05-27T11:37:13Z

khal/cli.py

@@ -341,6 +341,9 @@ def klist(ctx, include_calendar, exclude_calendar,
                  help=('Repeat event: daily, weekly, monthly or yearly.'))
    @click.option('--until', '-u',
                  help=('Stop an event repeating on this date.'))
+    @click.option('--every', '-e',
+                  help=('Frequency for repeating events.'),


Referring to this as "frequency" is likely to cause confusion as that's used in the code and the RFC for "weekly"/etc (FREQ)

Yeah, I agree, tried a few things different variations to make precise and concise and mixed it up in the final edit. The wording from the spec would be "positive integer representing at which intervals the recurrence rule repeats". "interval of repeating event"?

I initially looked at adding/updating tests but found some that were relevant were marked xfail so I was not sure how to proceed.

khal/controllers.py

d7415 · 2021-05-27T11:56:03Z

khal/controllers.py

@@ -413,6 +413,24 @@ def present_options(options, prefix="", sep="  ", width=70):
        return None


+def prompt_none(text, default=None, lower=False, type=None):


This "helper" feels like it's overcomplicating things to me. Especially with the two optional arguments that are only used once each - i.e. it's called 3 times, with 2 special cases.

I added the helper to eliminate the duplicate logic for the 3 attributes. This shrunk the code from 14 to 7 lines not counting the helper. Planning on using it to fix #1054 which would make it the 4th use, and it's duplicated against for the "else" attribute case and alarm (6 uses).

No love lost on the lower case conversion special case either, but it needs to happen after the type coercing from list to str. The better alternative is probably to split the function into two: 1. convert value, if list, to str. Any idea of name for this function? 2. "None" business then handle the case conversion for frequency inline.

In either case, I was looking for input on #1050 to see if we can eliminate the magic "None" value, so this just leaves the type conversion.

d7415 · 2021-05-27T12:17:11Z

khal/parse_datetime.py

+            try:
+                int(every)
+            except ValueError:


It would be clearer here (and more efficient, not that it really matters) to use if every.isdigit() here, as you're not using the cast.

Yes, later discovered that prompt had a type feature to constraint the input which is a better way of handling it, and later learned that it also have to be positive to I will have a change on that coming.

fwiw .isdigit() would return false for negatives :)

isdigit() ftw :-)

Well, turns out that stores values are type vInt so cannot use isdigit() on that, and I need to check for int(every) > 0. Just a heads up if you wonder why I didn't end up usign isdigit() after all.

khal/parse_datetime.py

khal/controllers.py

Co-authored-by: Martin Stone <[email protected]>

This includes support for interactive setting or editing an event.

Typo Co-authored-by: Hugo Barrera <[email protected]>

Fix #999. This also addresses a defect where editing a previously saved event cause the 3 rrule properties (freq, until and interval) to have incorrect default type (list instead of str), and freq value is forced to lower case as the current code requires it but more important that is how the values are presented in the interactive session.

Typo. Co-authored-by: Hugo Barrera <[email protected]>

Co-authored-by: Martin Stone <[email protected]>

Hugo Osvaldo Barrera and others added 2 commits May 26, 2021 10:15

Update IRC references to Libera.Chat

e88cce8

Add --every / -e option to the new command

b71d522

This includes support for interactive setting or editing an event.

WhyNotHugo reviewed May 26, 2021

View reviewed changes

doc/source/usage.rst Outdated Show resolved Hide resolved

doc/source/usage.rst Outdated Show resolved Hide resolved

khal/controllers.py Outdated Show resolved Hide resolved

Hugo Barrera and others added 4 commits May 26, 2021 17:20

Merge pull request #1051 from pimutils/irc

99f4e9d

Update IRC references to Libera.Chat

Update doc/source/usage.rst

fa85104

Typo Co-authored-by: Hugo Barrera <[email protected]>

Update doc/source/usage.rst

b5ecb5b

Typo Co-authored-by: Hugo Barrera <[email protected]>

WhyNotHugo approved these changes May 26, 2021

View reviewed changes

khal/controllers.py Outdated Show resolved Hide resolved

Update khal/controllers.py

b56be97

Typo. Co-authored-by: Hugo Barrera <[email protected]>

ghost mentioned this pull request May 27, 2021

new/edit description should be optional #1054

Open

d7415 requested changes May 27, 2021

View reviewed changes

allanwind and others added 17 commits May 27, 2021 10:15

Update khal/controllers.py

ed3f16d

Co-authored-by: Martin Stone <[email protected]>

Update khal/controllers.py

ca7b619

Co-authored-by: Martin Stone <[email protected]>

Update khal/parse_datetime.py

20ede9b

Co-authored-by: Martin Stone <[email protected]>

Add --every / -e option to the new command

fcae16b

This includes support for interactive setting or editing an event.

Update doc/source/usage.rst

e847c88

Typo Co-authored-by: Hugo Barrera <[email protected]>

Update doc/source/usage.rst

1693d52

Typo Co-authored-by: Hugo Barrera <[email protected]>

Update khal/controllers.py

1df179f

Typo. Co-authored-by: Hugo Barrera <[email protected]>

Update khal/controllers.py

5b5f08a

Co-authored-by: Martin Stone <[email protected]>

Update khal/controllers.py

e950299

Co-authored-by: Martin Stone <[email protected]>

Update khal/parse_datetime.py

11bc781

Co-authored-by: Martin Stone <[email protected]>

Fix #1054 - description should be optional

358af65

Parsing errors should not result in data loss

ccd19ca

Merge branch '999' of github.com:allanwind/khal into 999

fcdb1c4

Typo

d8b324c

Module 'click.types' has no attribute 'BadParameter'

b6cf6e9

Prompt should match option name

e36d0a9

geier added the type: enhancement label Jun 25, 2022

ghost closed this by deleting the head repository Sep 13, 2023

This pull request was closed.

		@@ -413,6 +413,24 @@ def present_options(options, prefix="", sep=" ", width=70):
		return None


		def prompt_none(text, default=None, lower=False, type=None):

Add --every / -e option to the new command #1052

Add --every / -e option to the new command #1052

Uh oh!

Conversation

ghost commented May 26, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

WhyNotHugo commented May 26, 2021

Uh oh!

d7415 left a comment

Choose a reason for hiding this comment

Uh oh!

d7415 May 27, 2021

Choose a reason for hiding this comment

Uh oh!

ghost May 27, 2021

Choose a reason for hiding this comment

Uh oh!

ghost May 27, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

d7415 May 27, 2021

Choose a reason for hiding this comment

Uh oh!

ghost May 27, 2021

Choose a reason for hiding this comment

Uh oh!

d7415 May 27, 2021

Choose a reason for hiding this comment

Uh oh!

ghost May 27, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

d7415 May 27, 2021

Choose a reason for hiding this comment

Uh oh!

ghost May 27, 2021

Choose a reason for hiding this comment

Uh oh!

ghost May 28, 2021 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ghost commented May 26, 2021 •

edited by ghost

Loading

ghost May 27, 2021 •

edited by ghost

Loading

ghost May 27, 2021 •

edited by ghost

Loading

ghost May 28, 2021 •

edited by ghost

Loading