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.

Sign up for GitHub

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

Jump to bottom

added the regular expression for windows paths #143

Merged
merged 3 commits into from
Jan 3, 2019
Merged

added the regular expression for windows paths #143

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45f6350
added the regular expression for windows paths
stukalin Nov 26, 2018
8f0a2e0
comment for the regex
stukalin Dec 12, 2018
dfd7d4f
better comment for the regex
stukalin Jan 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/service-worker-builder.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,10 @@ module.exports = class ServiceWorkerBuilder {

_rollupTree(tree, entryFile, destFile) {
let rollupReplaceConfig = {
include: ['/**/ember-service-worker/**/*.js'],
include: [
'/**/ember-service-worker/**/*.js',
/^[a-z]:\/(?:[^\/:*?"<>|\r\n]+\/)*ember-service-worker\/(?:[^\/:*?"<>|\r\n]+\/)*[^\/:*?"<>|\r\n]*\.js$/i
stukalin marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

It appears that you are checking for "/" in the regex rather than "" in the comment. Which one is the intended? I seem to recall windows paths being more like your example but it's been a while....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is true. Windows paths do have \ as path separators, but somewhere in the code all the paths are "normalized" to use only /s. I may fix the comment though. What would you say?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the silence... holidays and all. I'm trying to find where that happens but I'm coming up short. I would just make the comment as explicit as it needs to be so that the next person looking at this understands why the regex is the way it is. Maybe same as above but for windows paths like D:\my-ember\ember-service-worker\whatever\sw.js; which are updated before this step to D:/my-ember/ember-service/worker/whatever/sw.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eshtadc done!

],
delimiters: ['{{', '}}'],
ROOT_URL: this.options.rootURL
};
Expand Down