Skip to content
This repository has been archived by the owner on Sep 20, 2021. It is now read-only.

#21 Added support for embedded directions #23

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

#21 Added support for embedded directions #23

wants to merge 2 commits into from

Conversation

boast
Copy link
Contributor

@boast boast commented Jan 29, 2015

Edit by @Hywan: Fix #21.

@boast boast changed the title #21 Added support for embedded directions Added support for embedded directions Jan 29, 2015
@boast boast changed the title Added support for embedded directions #21 Added support for embedded directions Jan 29, 2015
@Hywan Hywan self-assigned this Jan 29, 2015
$this->_direction = static::LTR;

// Check for LRM or RLM/ARM
$hasLRM = 0 !== preg_match('#\x{200e}#u', $this->_string);
Copy link
Member

Choose a reason for hiding this comment

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

I prefer to use constant for LRM & co. in the regular expression :-).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem here is, that the already defined static constant (LRM, RLM, ARM, etc) cannot be easily included in the regular expression, as they are only integers and regular expression use a \x{codepoint syntax - given the u modifier at the end. So no easy solution for that..

Copy link
Member

Choose a reason for hiding this comment

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

'#\x{' . dechex(self::LRM) . '}#u', thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

why not :) ill change it accordingly

@Hywan
Copy link
Member

Hywan commented Jan 29, 2015

Thanks a lot for this PR \o/.

mb_substr($this->_string, 0, 1)
);
// Default
$this->_direction = static::LTR;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Set Default Direction to LTR

@Hywan
Copy link
Member

Hywan commented Mar 26, 2015

ping?

@Hywan
Copy link
Member

Hywan commented Aug 3, 2015

@boast ping?

1 similar comment
@Hywan
Copy link
Member

Hywan commented May 11, 2016

@boast ping?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

Support embedded directions
2 participants