Skip to content

Allow array of trees#188

Open
SparshithNR wants to merge 3 commits intobroccolijs:masterfrom
SparshithNR:allow-tree-array
Open

Allow array of trees#188
SparshithNR wants to merge 3 commits intobroccolijs:masterfrom
SparshithNR:allow-tree-array

Conversation

@SparshithNR
Copy link

@SparshithNR SparshithNR commented Dec 19, 2019

  • Avoids need for merging trees before calling Filter
    This change will do the following code work
const Filter = require('broccoli-persistent-filter');
//BrocFile.js

class Awk extends Filter {
  constructor(inputNode, search, replace, options) {
    options = options || {};
    super(inputNode, {
      annotation: options.annotation
    });
    this.search = search;
    this.replace = replace;
    this.extensions = ['txt'];
    this.targetExtension = 'txt';
  }
  processString(content, relativePath) {
    return content.replace(this.search, this.replace);
  };
}
//before current change we use to run a mergeTree and then run Awk filter
// i.e. let mergedTree = new MergeTree(['fixture','fixture_2']);
// module.exports = new Awk(mergedTree, 'test', 'real');
// now we can pass whole array of inputTrees 
module.exports = new Awk(['fixture','fixture_2'], 'test', 'real');

@SparshithNR
Copy link
Author

This PR is the next step to #187 .

@SparshithNR
Copy link
Author

SparshithNR commented May 12, 2020

@chriseppstein @stefanpenner Please have a look at this change.

Copy link
Contributor

@chriseppstein chriseppstein left a comment

Choose a reason for hiding this comment

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

Looks good. I have raised one serious issue regarding the error metadata being returned.

@SparshithNR SparshithNR force-pushed the allow-tree-array branch 2 times, most recently from 14e3ce8 to e729381 Compare June 5, 2020 15:48
sparshithNR and others added 3 commits June 25, 2020 10:31
Co-authored-by: Chris Eppstein <chris@eppsteins.net>
@chriseppstein
Copy link
Contributor

@SparshithNR @stefanpenner Is there anything remaining to do in order to land this? It would be good if we could realize the gains of all the underlying changes.

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