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 period and colon in section ids. #3647

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

trollkotze
Copy link
Contributor

A verbose explanation why including colons and periods in section IDs should be allowable and does not cause problems can be found in this 3 year old issue: #2580
Running a DokuWiki with this small modification since 3 years, allowing '.' and ':' in section ids and anchor links to them etc. has caused no problems in all this time.

A verbose explanation why including colons and periods in section IDs should be allowable and does not cause problems can be found in this 3 year old issue: dokuwiki#2580
Running a DokuWiki with this small modification since 3 years, allowing '.' and ':' in section ids and anchor links to them etc. has caused no problems in all this time.
@Klap-in
Copy link
Collaborator

Klap-in commented Mar 21, 2022

I think it is logic to allow these characters.

Could you please extend the unit tests?
https://github.com/splitbrain/dokuwiki/blob/master/_test/tests/inc/pageutils_sectionid.test.php

Should we consider (breaking) backward compatibility? Some existing links with these characters will break..

@trollkotze
Copy link
Contributor Author

@Klap-in Sorry, I'm not familiar with your unit tests so am not sure if I understand all around correctly.
Since the change in functionality here is just to allow '.' and ':' in section ids, I guess one would just change the test sequences from

            [['A', 'A', 'A1']],
            [['A', 'A1', 'A']]

to something including colons and dots? I would appreciate if you could take this up since you probably understand better what to test for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants