Skip to content

feat: add repair step for missing multiline indentation - #920

Open
pmeier wants to merge 1 commit into
nextcloud:mainfrom
pmeier:repair-multiline-indent
Open

feat: add repair step for missing multiline indentation#920
pmeier wants to merge 1 commit into
nextcloud:mainfrom
pmeier:repair-multiline-indent

Conversation

@pmeier

@pmeier pmeier commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

RFC 5455 specifies in Section 3.1:

a long line can be split between any two characters by inserting a CRLF immediately followed by a single linear white-space character (i.e., SPACE or HTAB)

One of my calendar apps dropped a multiline location without any indentation, which without this PR cannot be parsed:

LOCATION:Some Company
Some Street
Some Town

This PR adds a repair step that finds unindented multilines and indents them. So after the repair is applied, the output is

LOCATION:Some Company
 Some Street
 Some Town

*/
repair(ics) {
return ics
.replace(/^(?![ \t]|[0-9a-zA-Z-]+(?:;[^:]*)?:)(.+)$/gm, (_, unindentedLine) => ` ${unindentedLine}`)

@pmeier pmeier Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Happy to add my explanation as a comment, but I saw that the other repair steps didn't have much comments either. So here it is.


We are using a negative lookbehind regex (?!...) to only match on lines that don't fall into one the following categories:

  • The line is indented either by simple space or a tab [ \t], or
  • The line is a property or block delimiter [0-9a-zA-Z-]+: optionally with key-value pairs (;[^:]*)? (the (?:...) is a non-capturing group since we don't care about the content)

If we find such a line and it is not empty (.+), we prepend a space to it.

@SebastianKrupinski SebastianKrupinski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @pmeier

Thank you for the PR. Sorry but I am going to have to block this PR. we should not be deviating from the RFC standard just to fix a third party apps formatting issues.

@pmeier

pmeier commented Aug 24, 2026

Copy link
Copy Markdown
Contributor Author

@SebastianKrupinski Not sure I understand your comment or I'm missing the point of the repair steps. The RFC states that any multiline values need to be indented. Some clients, like the one I used, failed to do this. So to be able to parse an ICS file from such a client, it needs to be repaired.

The repair step proposed in this PR is not deviating from the RFC at all. The complex regex is specifically designed to not match any lines that are valid and thus only fix the ones that need repairing in the first place.

Signed-off-by: Philip Meier <github.pmeier@posteo.de>
@pmeier
pmeier force-pushed the repair-multiline-indent branch from e151d0e to 7eb5745 Compare August 24, 2026 15:13
@SebastianKrupinski

Copy link
Copy Markdown
Contributor

Some clients, like the one I used, failed to do this. So to be able to parse an ICS file from such a client, it needs to be repaired.

Hi, so this is my point exactly, the issue is a broken client not the NC software. Also implementing this here, only fixes the UI, the event is still broken for any other client that opens it.

I would suggest that you put in a request or PR to fix the client software.

@pmeier

pmeier commented Aug 26, 2026

Copy link
Copy Markdown
Contributor Author

the event is still broken for any other client that opens it.

But I don't want to do that? I'm currently onboarding a user onto my NC. They currently use another calendar that has the broken behavior I explained above. I want to migrate their current calendar as ICS file to NC using the import function the NC calendar app provides. This currently fails. And unless I'm mistaken the repair steps apply to this exact scenario:

* @classdesc A repair step is used to fix calendar-data before it is parsed

I'm a little confused about the pushback TBH. I've contributed a repair step before with basically the exact same setup, i.e. broken ICS from another client cannot be parsed, and there my patch was fine. What difference is there to this PR that am I missing?

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