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

Another attempt at fixing combinations of em and links #597

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

Conversation

michitux
Copy link
Collaborator

@michitux michitux commented Mar 8, 2014

As requested by Julian Fagir (gnrp) in the irc channel I'm proposing this fix for the italics/em syntax.

This fixes cases with "normal" URLs but creates some weird special cases:

  • this triggers // italics [[http://ärmel.de]] some test (this does not trigger // italics [[http://www.ärmel.de]] some test)
  • //no italics://this is neither a url nor italics but //italics://this triggers italics because there are //more slashes later

@splitbrain
Copy link
Collaborator

can we have test cases for this?

@Klap-in
Copy link
Collaborator

Klap-in commented Mar 8, 2014

(related old bugs: FS#2017, FS#1795 and FS#384)

@Chris--S
Copy link
Collaborator

Chris--S commented Mar 8, 2014

Not sure I like the regex. How about this

//(?=.*(?:(?://\s)|(?:(?<!:)//)))

translated.

looking for a terminating pair of slashes where the slashes are followed by white-space or not preceded by a colon. so scheme://domain will not terminate, but both foo:// bar & foo//bar will.

update:

the \s may need to be replaced by a character list to avoid utf8 bytes which can look like spaces under some locale settings (I forget which FS# addressed that, and if it applied to strings inside the parser).

update2
maybe the above regex can be simplified to

//(?=.*(?://\s|[^:]//))

@splitbrain
Copy link
Collaborator

This definitely needs test cases before it's ready to merge

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
PSR-2 Finishing
  
Triage
Development

Successfully merging this pull request may close these issues.

None yet

4 participants