Provide way to add options to your mapping #4
Conversation
youwe-petervanderwal
requested review from
rutgerrademaker,
Anve94,
elzekool,
igorwulff and
leonhelmus
| @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. | |||
| The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) | |||
| and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). | |||
|
|
|||
| ## 2.0.0 | |||
There was a problem hiding this comment.
Had to go with a major version bump because of the change in the interface
There was a problem hiding this comment.
Maybe also remove PHP 7 compatibility then? See composer.json
youwe-petervanderwal
requested review from
kjkooistra-youwe,
eescribanoc and
MartijnVanDoornik
| |----------------------------------|----------------------------|--------------------------------| | ||
| | `file.php` | `file.php` | `file.php` | | ||
| | `{dot,.}gitignore` | `dotgitignore` | `.gitignore` | | ||
| | `{default/,}config.yaml{.dist,}` | `default/config.yaml.dist` | `config.yaml` | |
There was a problem hiding this comment.
I would like to suggest to use the | character. Not only is this a very common character in substitution formats it is also a character which you will never find in file names as it is reserved in both *nix and Windows based systems.
There was a problem hiding this comment.
I'm only adding documentation here. This (with a comma) is how the mapping files already works (see https://github.com/YouweGit/testing-suite/blob/master/templates/mapping/files as example).
The actualy only change I introduce in this PR is the feature to provide options to the mapping, where I use the : separator (already a prohibited filename character in Windows -- UNIX allows a lot).
Although I do agree with you that | would have been a better choice than the , -- I think that changing that might be a bit out-of-scope and will require more refactoring
| destinationDirectory: getcwd(), | ||
| mapping: '{templates/dot,.}gitignore' | ||
| 'option1', | ||
| 'option2' |
| sourceDirectory: __DIR__ . '/../folder/files', | ||
| targetDirectory: getcwd(), | ||
| 'path/to/mapping-file-1', | ||
| 'path/to/mapping-file-2' |
| @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. | |||
| The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) | |||
| and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). | |||
|
|
|||
| ## 2.0.0 | |||
There was a problem hiding this comment.
Maybe also remove PHP 7 compatibility then? See composer.json
| /** | ||
| * @var string[] | ||
| */ | ||
| private $options; |
There was a problem hiding this comment.
Add typehint (depending on whether PHP 7 compatibility is removed)
| return new UnixFileMapping( | ||
| $this->sourceDirectory, | ||
| $this->targetDirectory, | ||
| trim($mapping) | ||
| $mapping, | ||
| ...$options |
There was a problem hiding this comment.
Trailing comma.
Should options be trimmed?
| ], | ||
| 'files' => | ||
| "{foo,bar}.php\n" . | ||
| "{templates/dot,.}gitignore:merge:force\n", |
There was a problem hiding this comment.
Should this break to the next line? And if so, shouldn't it be indented one more level?
See YouweGit/composer-file-installer#5