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

Improvement to support setting the regex per file group. #168

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

Conversation

MikeSpock
Copy link

Implementation for this feature request: #131

Improvement to support setting the regex per file group. Backwards compatible with the old notation.

New Notation:

files: [
  {
    path: 'index.html',
    regexp: /(dist\/\w+\/\w+[-](([\d-]+\.)+[\d-]+))/g
  },{
    path: ['package.json', 'bower.json']
  }
]

Old Notation:
files: ['package.json','index.html', 'bower.json']

If no regexp property is defined within the file, the script falls back to use the regExp prop from the bump config object. If regExp prop is not defined, it uses the built-in default regexp.

Please mind, that I didn't refine the regex matching logic within the code, so you'll have to be careful with your regexes, and mind that the code expects to get the match in a strictly defined order. Use --dry-run to check if everything is ok.

…mpatible with the old notation.

New Notation:
files: [
                    {
                        path: 'index.html',
                        regexp: /(dist\/\w+\/\w+[-](([\d-]+\.)+[\d-]+))/g
                    },
                    {
                        path: ['package.json', 'bower.json']
                    }
                ]

Old Notation:
files: ['package.json','package.json', 'bower.json']

If no regexp property is defined within the file, the script falls back to use the regExp prop from the bump config object. If regExp prop is not defined, it uses the built-in default regexp.

Please mind, that I didn't refine the regex matching logic within the code, so you'll have to be careful with your regexes, and mind that the code expects to get the match in a strictly defined order. Use --dry-run to check if everything is ok.
@@ -44,6 +44,40 @@ grunt.initConfig({
})
```

Alternatively, you can use different regexes to your target files:
Copy link
Collaborator

Choose a reason for hiding this comment

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

this should be explained the files option section, not the defaults section

Change requested by @eddiemongle in vojtajina#168
@MikeSpock
Copy link
Author

any update on thes?

@eddiemonge
Copy link
Collaborator

I'm not sure I like the api for this. I think its a bit too complicated. I think the files array might be better as accepting strings and objects. If its a string, it behaves like it does now. If its an object, it requires path (as a string) and a regexp. The code itself is also harder to follow in this.

Also, the commits should be squashed and changed to follow the contributing guidelines

}
]
```
The example above replaces urls like `dist/js/app-1.0.1.js` or `dist/css/style-1.0.1.css`
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is not a good example. Grunt-bump shouldn't be used for this since its not renaming those files.

This can be useful for replacing versions in non-JSON files. Although that is part of bigger change I want to make where you specify the key in a JSON file to replace the version with and then the regex is only for nonJSON files

@MikeSpock
Copy link
Author

I can squash the commits no problem, in case you'd accept this solution. Let me know if you're planning to use this or not, cause if you do, I'll do the changes.

My usecase for this was that my grunt job creates dist files in a versioned folder: dist/1.9.1/js/app.min.js and dist/1.9.1/css/app.min.css, etc, so when the grunt bump happens, I want to change the paths in my index.html.

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