Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow organiser to contain comma, ensure that new lines in values don't break output, and various other tweaks (take 2) #261

Merged

Conversation

tjenkinson
Copy link
Contributor

Thanks for this great library!

We had a bug where we were providing an organiser where the name included a ,, and the fix was to wrap the name in quotes. Essentially take the quoted-string option for param-value in the spec

So the I decided to make the same change to the other param-value's so that they don't have the same issue.

And then I added some logic to encode new lines to\n in a lot more places because otherwise it's possible for user input containing new lines to break things

And as I was going through I spotted a few other things so here's a list of all the tweaks/fixes:

  • exclusionDates was missing in the schema so I don't think it could be used before? Now it can be and I changed it to be an array of date/time entries
  • date time entries can now be a single number which is the unix time, which is easy to get from date.getTime(), or a string which is then just passed through
  • the default values are now built with a fn at runtime as this means the default date will be the invoke time and not the time when the library was loaded, and also means the uuid is unique every time and assignUniqueId was no longer needed
  • some of the regex patterns in the schema check weren't bounded. I.e. oopsCANCELLED would be allowed
  • I split up the event into header/event/footer (the second commit). This makes it a bit cleaner when there are no events as don't need to do any string searching anymore. It still supports taking the calendar name etc from the first event object, but in the future it might be worth updating the public api to support passing the header fields separately to the event ones, so that then if you are creating a calendar feed where the are no events it's still possible to set the header fields and not get the defaults
  • Added an optional second param to getEvents which can take header attributes, so it's possible to still have the correct header attributes even when there are no events
  • added encodeParamValue which is used to encode param-value's. The spec doesn't appear to say that "'s are allowed in the string, but escaping them with \ does seem to work when I tested in the apple calendar app

@adamgibbons
Copy link
Owner

@tjenkinson seeing just this one test failing now (Node v18):

Screen Shot 2023-12-13 at 10 26 57 AM

@tjenkinson
Copy link
Contributor Author

Ah I think this may be a timezone thing. Will prepare a pr

@adamgibbons
Copy link
Owner

Ah I think this may be a timezone thing.

If I had a dollar for every time i said 👆...

@tjenkinson
Copy link
Contributor Author

Yeh timezones are a nightmare #262

renovate bot added a commit to inabagumi/shinju-date that referenced this pull request Dec 14, 2023
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [ics](https://togithub.com/adamgibbons/ics) | [`^3.6.2` ->
`^3.7.1`](https://renovatebot.com/diffs/npm/ics/3.6.2/3.7.1) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/ics/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/ics/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/ics/3.6.2/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/ics/3.6.2/3.7.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>adamgibbons/ics (ics)</summary>

###
[`v3.7.1`](https://togithub.com/adamgibbons/ics/compare/v3.7.0...ecc8cf54f84da92b3a0fe0fa049fc65cda4d1451)

[Compare
Source](https://togithub.com/adamgibbons/ics/compare/v3.7.0...ecc8cf54f84da92b3a0fe0fa049fc65cda4d1451)

### [`v3.7.0`](https://togithub.com/adamgibbons/ics/releases/tag/v3.7.0)

[Compare
Source](https://togithub.com/adamgibbons/ics/compare/v3.6.3...v3.7.0)

#### What's Changed

- fix(utils): 🐛 fixed the issue with foldLine not correctly handling
strings containing emojis. by
[@&#8203;haydenull](https://togithub.com/haydenull) in
[adamgibbons/ics#258

#### New Contributors

- [@&#8203;haydenull](https://togithub.com/haydenull) made their first
contribution in
[adamgibbons/ics#258

**Full Changelog**:
adamgibbons/ics@v3.6.3...v3.7.0

### [`v3.6.3`](https://togithub.com/adamgibbons/ics/releases/tag/v3.6.3)

[Compare
Source](https://togithub.com/adamgibbons/ics/compare/v3.6.2...v3.6.3)

#### What's Changed

- Allow `organiser` to contain comma, ensure that new lines in values
don't break output, and various other tweaks (take 2) by
[@&#8203;tjenkinson](https://togithub.com/tjenkinson) in
[adamgibbons/ics#261
- Fix exception date-time test by
[@&#8203;tjenkinson](https://togithub.com/tjenkinson) in
[adamgibbons/ics#262

**Full Changelog**:
adamgibbons/ics@v3.6.2...v3.6.3

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/inabagumi/shinju-date).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy44Ny4yIiwidXBkYXRlZEluVmVyIjoiMzcuODcuMiIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

None yet

2 participants