Allow template substitutions in scaffold files

Oh, wow. This would be really great!

Nice! Also “27,404 pass”, hehe

Test fail appears to be a random fail - but there are 8 cs issues with the patch. I've requeued to verify the random fail.

I know we are not changing the project root file here but there are some lock file updates missing if you run composer update drupal/core . Can we add those updates here as well?

-  protected function ensureLocations() {
+  protected function getVendorRelativePath() {
+    $vendor_dir = $this->composer->getConfig()->get('vendor-dir');
     $fs = new Filesystem();
-    $locations = $this->getOptions()->locations() + ['web_root' => './'];
-    $locations = array_map(function ($location) use ($fs) {
-      $fs->ensureDirectoryExists($location);
-      $location = realpath($location);
-      return $location;
-    }, $locations);
-    return $locations;
+    return $fs->findShortestPath(getcwd(), $vendor_dir);
   }

It looks like the ensureLocations() method was removed and its array_map operation not moved elsewhere. Could you provide a little background on how that's safe / why it's no longer needed?

I've reviewed the code and extensively tested this. The same path determination and interpolation that's been modified here to meet the needs of template substitution was already being used to determine actual destination paths where scaffolded files should be layed down on the disk, and so my greatest concern was that we might cause regressions with that, but I have been unable to find any issues with the scaffolded files being placed in the wrong location. In particular, composer --working-dir invocations still place scaffold files correctly.

However, I've tested using this with a modified `drupal/core` that includes interpolation instructions for a scaffolded .gitignore, and discovered it does not quite meet our needs for #3082958: Add gitignore(s) to Composer-ready project templates. The problem is that we are defining the location of the web-root to be . for LegacyProject, which gets rendered out to the .gitignore files. Surprisingly, a . anywhere in a gitignore rule breaks it. E.g., git does not treat . in .gitignore paths like Linux/Windows does.

There is no value we can provide for web-root that both causes the scaffold files to write out to the correct location AND generates a working .gitignore.

I've reviewed #17. It knowingly does some massaging of the template data that you wouldn't ordinarily expect a template substitution system to do in order to solve the specific problem raised in #16. In my opinion, this is okay, because, like all scaffolding operations, these new template substitutions are configurable. This will allow us to start with the current behavior and if needed add support for more generalized templating in future by requesting it through to-be-added configuration.

Also did an end-to-end test of #17 to verify it solves our "templatize .gitignore" use case.

New changes in #19 make sense so back to RTBC. Rebased the patch as well.

+++ b/composer/Plugin/Scaffold/Interpolator.php
@@ -88,16 +88,89 @@ public function addData(array $data) {
+  public function interpolate($message, array $extra = [], $default = '', $fix_paths = false) {

I have been seeing the usage of constants such as `true` and `false` in lower case in past few patches. Have our coding standards changed? The page at https://www.drupal.org/docs/develop/standards/coding-standards still shows that they have to be upper case.

+++ b/composer/Plugin/Scaffold/Interpolator.php
@@ -88,16 +88,89 @@ public function addData(array $data) {
+  protected function fixDoubleDirectorySeparators($replacements) {
+    $result = [];
+    foreach ($replacements as $path => $value) {

Isn't it better to just str_replace("//", "/", $path)? Is there a use-case where doing all this work is needed?

Ah, got it. We are not parsing the gitignore file, we are just replacing it in the whole file contents.

1. +++ b/composer/Plugin/Scaffold/Interpolator.php
   @@ -88,16 +88,89 @@ public function addData(array $data) {
   +   * @param bool $fix_paths
   +   *   TRUE to ensure that [web-root]/index.php is rendered as ./index.php, and
   +   *   not as /index.php.
   ...
   -  public function interpolate($message, array $extra = [], $default = '') {
   +  public function interpolate($message, array $extra = [], $default = '', $fix_paths = FALSE) {
   

   Why is the default FALSE? Or to put this question a different way - why does interpolatePath call interpolate? I think we could copy the interpolate lines into interpolatePath and also put fixDoubleDirectorySeparators() inline and we'd have less API change and less API to support in the future - ie. other code that calls interpolate never has to consider whether it should set $fix_paths to TRUE.

2. +++ b/composer/Plugin/Scaffold/Operations/OperationData.php
   @@ -90,6 +91,16 @@ public function overwrite() {
   +  /**
   +   * Whether we are going to interpolate replacements in a scaffold file.
   +   *
   +   * @return bool
   +   *   Returns true if interpolation is desired.
   +   */
   +  public function interpolate() {
   

   I was wondering is this should be isInterpolated(). Existing methods on the OperationData class are like

     /**
      * Determines overwrite.
      *
      * @return bool
      *   Returns true if overwrite mode was selected.
      */
     public function overwrite() {
       return !empty($this->data[self::OVERWRITE]);
     }
   
    /**
      * Checks if prepend path exists.
      *
      * @return bool
      *   Returns true if prepend exists.
      */
     public function hasPrepend() {
       return isset($this->data[self::PREPEND]);
     }
   
     /**
      * Gets prepend path.
      *
      * @return string
      *   Path to prepend data
      */
     public function prepend() {
       return $this->data[self::PREPEND];
     }
   

   So I guess this is okay but it's a bit confusing given it's not interpolating.

Back to RBTC as #27 is addressed.