Skip to content

Collect optional 'match' from config and use it to parse ical#649

Draft
isaaclw wants to merge 5 commits intolanguitar:mainfrom
isaaclw:main
Draft

Collect optional 'match' from config and use it to parse ical#649
isaaclw wants to merge 5 commits intolanguitar:mainfrom
isaaclw:main

Conversation

@isaaclw
Copy link

@isaaclw isaaclw commented Apr 21, 2025

Check the configuration for an optional string: 'match'

If the string exists anywhere in the summary, then the event is valid.
If the string is missing in the config entirely, then all events are valid

See #340

Check the configuration for an optional string: 'match'
If the string exists anywhere in the summary, then the event is valid.
If the string is missing in the config entirely, then all events are valid
@isaaclw
Copy link
Author

isaaclw commented Apr 22, 2025

Sorry, there's a bug in this, The code needs to be duplicated in the class Calendar also.

I'm fixing it and testing again.

@isaaclw isaaclw marked this pull request as draft April 22, 2025 11:29
Copy link
Owner

@languitar languitar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking up on this one

A few general things before going into details:

  • Please follow the commit convention https://www.conventionalcommits.org/en/v1.0.0/
  • The documentation also needs an update
  • I'd be in favor of using a regular expression for match and not simply substring matching. That gives a few more options.
  • We'd need tests for the new feature.

Match may be 'none' or a string.

Co-authored-by: Johannes Wienke <languitar@semipol.de>
@isaaclw
Copy link
Author

isaaclw commented Apr 23, 2025

Thanks for picking up on this one

A few general things before going into details:

* Please follow the commit convention https://www.conventionalcommits.org/en/v1.0.0/

Hm. I'm following the guide as close as I can. Unfortunately I can't edit previous commits without rebasing, and rebasing is "against the law" so I'm not sure what to do there. Maybe when it's merged the commit could be clarified as completely as possible using the commit convention?

If you have specific suggestions regarding the commits, then I can try to do better next time.

* The documentation also needs an update

Is that in the repo? I can try to flesh that out.

* I'd be in favor of using a regular expression for match and not simply substring matching. That gives a few more options.

Regular expression also makes it messier. To be honestly I was thinking I would just do 'starts with' string, but I thought contains would be better. I'll accept a pull request regarding the regular expression though ;)

* We'd need tests for the new feature.

Hm. ok, I'll see if I can give this a shot, but this is starting to look like a local fix that I'll just roll myself if there's too many barriers to merge it in.

Thanks for your reply. I'm still trying to get my host to respond clearly to this code change so I can be sure this is working.

@languitar
Copy link
Owner

Sorry for the delay.

Rebasing is perfectly fine on a feature branch. You can freely rebase here to get the commit messages right.

For the rest: give me a ping if you need help. I could take over the rest once you're ready with everything you've done.

@isaaclw
Copy link
Author

isaaclw commented Jul 9, 2025

Sorry, I've put this on hold with other projects that came up, and my use case to test this died... the PSU died, and it's not worth replacing. Thanks for the work you've been doing and I'll try to come back to it at some point if I can.

@languitar
Copy link
Owner

Thanks for the hint. I had actually started fixing the remaining bits and pieces here but also got distracted by other things.

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.

2 participants