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

Change REGEX_RE to allow use of / in filters #886

Merged
merged 1 commit into from
Mar 20, 2019

Conversation

Etiennef
Copy link

@Etiennef Etiennef commented Feb 17, 2019

In mutation or event filtering, it is currently possible to filter using regexp, but not to use the / char in a regex.
This prevents to filter on namespaced mutation name, which is sad. My personnal example is that I wanted to filter out the mutation (core/logger/log) with the following regexp : /^((?!core\/logger\/log).)*$/ and it didn't work. Filtering with /^((?!logger).)*$/ did work as expected, but filters too much.

I checked the code in src/devtools/views/vuex/module.js an src/devtools/views/action/module.js
The regexp REGEX_RE = /^\/(.*?)\/(\w*)/ cuts the regexp at the first / encountered, making it impossible to use this char in a regexp, no matter how we try to escape it.

I see several ways to do solve it:

  • I'm not sure why the .* in REGEXP_RE is not greedy. removing the ? seems to work : REGEX_RE = /^\/(.*)\/(\w*)/
  • Same with the absence of $ at the end. If we assume there only is a regexp in the field (I don't know vue-devtools enough to know it it's a reasonable assumption) : REGEX_RE = /^\/(.*?)\/(\w*)$/.
  • Allow escaping it with \ as we would in javascript code. I belive the right regexp for this would be REGEX_RE = /^\/((?:(?:.*?)(?:\\\/)?)*?)\/(\w*)/

I implemented the last solution, which seems cleaner because closer to the initial design that I may not fully understand, and thus less likely to have side effects. It's also closer to the javascript syntax.

I'm not 100% sure of my regexp choice, but I tested it in what seemed to be representative cases (https://gist.github.com/Etiennef/2ef33b57fc7b63e730db050f8b905ce1)

@Etiennef Etiennef marked this pull request as ready for review February 17, 2019 15:16
@Akryum Akryum merged commit 144a4e6 into vuejs:dev Mar 20, 2019
simsim0709 pushed a commit to simsim0709/vue-devtools that referenced this pull request May 7, 2019
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