SBoudrias / mem-fs-editor

File edition helpers working on top of mem-fs (https://github.com/SBoudrias/mem-fs)
MIT License
415 stars 47 forks source link

New feature: Append #22

Closed olsonpm closed 7 years ago

olsonpm commented 9 years ago

I'm going to use read -> write as a workaround for now, but I was wondering whether you'd be open to merging a PR with this feature.

SBoudrias commented 9 years ago

Can you describe your use case that'd need an append feature rather than read/write

olsonpm commented 9 years ago

Sure thing.

I have a 'git' yeoman generator which will create a .gitignore file among other things. Other generators used via composeWith might append text to an existing .gitignore file

SBoudrias commented 9 years ago

What would happen if the file doesn't exist?

olsonpm commented 9 years ago

The library comes with an exist method - I figured I'd use that.

SBoudrias commented 9 years ago

I mean, what would be the behavior.

olsonpm commented 9 years ago

I would want it to append the text if the file exists, and not append if the file doesn't exist (the assumption being git is not set up). This seems logical to me mostly because scaffolders should not be run multiple times on the same project. The alternative solution is to have that logic sit in the git generator, which won't scale well.

SBoudrias commented 9 years ago

See, if we try appending to the file and it doesn't exist, it should at least throw an error.

The more I think about this, the more I don't feel it is necessary. Manually doing it doesn't feels like a big overhead, and the API from mem-fs-editor would probably be a bit weird depending on how we handle edge cases.

olsonpm commented 9 years ago

Ah I misunderstood your question on what the behavior would be. I was saying in my generator I would first see whether the file exists before using append. I agree the implementation of append would throw an error if it doesn't exist, just as read does. I agree too that read->write is not a difficult workaround, and that append would basically be a convenience method, thus not at all necessary.

I appreciate the feedback

SBoudrias commented 9 years ago

I might reconsider this feature. I can see it might be a common pattern in some programming languages tools.

I'll reopen for now.

llafuente commented 8 years ago

I'm also in need of append. My particular use case is run staff in order regardless the minify process that happen later, so everything should be in the same file.

But also found that ejs is only "exposed" in copyTpl. I think that append it's not necessary if you add an API to execute a template that we could previously read. Something like: execTpl

So people manually append the strings and write. That will give people full control.

this.write('xxx', this.execTpl(this.read('y1')) + this.execTpl(this.read('y2')))

thoughts ?

SBoudrias commented 8 years ago

@llafuente can't you just use ejs directly yourself?

llafuente commented 8 years ago

I could do something like: require("xxx/node_modules/ejs") i think.

But I dont won't to worry about ejs being replaced, or change their API, I prefer you to expose the same functionality that copyTpl has and will have.

SBoudrias commented 8 years ago

No no, you want to import yourself ejs from your own library. That way the version is the one you decide upon.

If yeoman mirror ejs to you, then when we bump it your code could break without you knowing where this is coming from.

llafuente commented 8 years ago

That's exactly what i dont want, two different ejs versions. Part of my templates using copyTpl and the other using ejs directly.

trainerbill commented 8 years ago

+1 for append. have some subgenerators that progressively build the README.md. Would also need appendTpl. My current solution is to copyTpl to a temp file, move README.md to a temp file, use write(read+read), and then remove the temp files. Would be willing to listen to any alternative as this is pretty messy.