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

Fix and simplify org name regex #60

Closed

Conversation

andylolz
Copy link
Contributor

@andylolz andylolz commented Feb 5, 2018

Adds a test that breaks; fixes it.

It’s worth bearing in mind that the regex matcher doesn’t have the DOTALL flag set in either python or PHP, so you have to be careful when using it for text nodes like narrative/text(), that may contain newlines. This actually (sort of) breaks ^(?!\s*$).+, for instance.

Refs #61; #56.

@andylolz andylolz force-pushed the fix-and-simplify-regex branch from d164750 to ca73931 Compare February 5, 2018 23:30
@andylolz andylolz changed the title Fix and simplify regex Fix and simplify org name regex Feb 5, 2018
@andylolz andylolz force-pushed the fix-and-simplify-regex branch from a48d52f to bfdf823 Compare February 5, 2018 23:36
@andylolz andylolz force-pushed the fix-and-simplify-regex branch from bfdf823 to 0c8f02b Compare February 5, 2018 23:37
@andylolz andylolz mentioned this pull request Feb 6, 2018
2 tasks
@andylolz
Copy link
Contributor Author

andylolz commented Feb 6, 2018

As for #59, this rule could equally be expressed by moving the conditional part out of the selector, and into a condition. For #59, the rule looks cleaner using a condition. For this one, I’m not so sure (I guess it would probably need to be split up into a couple of rules.)

findall can’t cope with conditions, which are used in the
org narrative rules. XPath is required.
@hayfield
Copy link
Contributor

hayfield commented Feb 14, 2018

The decided course of action is to not add this as a Machine Actionable Rule since the benefit is tiny (it doesn't really check anything super useful) given the complication in adding it.

@hayfield hayfield closed this Feb 14, 2018
@andylolz andylolz deleted the fix-and-simplify-regex branch February 14, 2018 12:29
@andylolz andylolz restored the fix-and-simplify-regex branch February 15, 2018 15:41
@andylolz andylolz deleted the fix-and-simplify-regex branch January 24, 2019 16:34
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