Inventsable / bombino-commands

Adobe CEP utility commands to supercharge dev workflow available through Node scripts or via terminal
5 stars 4 forks source link

Unwanted character escaping behavior for .certignore file #6

Closed hanezu closed 1 week ago

hanezu commented 4 years ago

First of all, amazing project! I have been using it for a while and most of the times it works quite well.

I found a bug related to this line.

By default, ^(\.git) is in .certignore file, so as a result of the above line, it is transformed into \^(\.git), which matches something like ^.git instead of .git. So .git is accidentally included in the signed package.

(BTW replace function actually only replace the first occurrence. So something like abc(def) becomes abc\(def), leading to regex compilation error.)

However, instead of fixing it by escaping all the characters, I wonder if it is better to just allow regex in .certignore file.

In my case, I wanted to ignore everything in node_modules/ EXCEPT node_modules/my-package. Currently I achieve it by commenting out that line and adding node_modules\/(?!my-package) to .certignore, which works well.

If you also think it's a good idea to enable regex, I would be willing to send a pull request on it.

Inventsable commented 4 years ago

Thanks!

I'd actually always meant for it to support regex, and at one time this was working (hence the regex syntax for ^.git). To be honest I've also been meaning to do a complete overhaul of bombino and it's commands so that everything fits into a single config file instead of needing multiple for certignore, signing information, and .bombino files. I was thinking it would be a bombino.config.js file that would look something more like this:

export default {
  certignore: [
    /^git/,
    "node_modules/"
  ],
  signInfo: {
     author: "Tom Scharstein",
     password: "123abc"
   }
}

And so on. I'd definitely appreciate a pull request to fix this on your end, but will be looking into revamping bombino so that it doesn't have the same legacy code from older generators (specifically for the signing function) and could also support UXP extensions instead of only CEP.

hanezu commented 4 years ago

Glad to hear that you already working on something like it! A single bombino.config.js sounds great. Although I guess some people might not want to store all config including sensitive information into version control, it should be easy to add an option for that. I'm fine working on a fork of this repo for now, so I am looking forward to the new bombino.😃

Inventsable commented 4 years ago

At the moment my main focus is on the brutalism component library for Adobe, which slots really easily into bombino and you can find in the template arrays, and a few other large projects I'm running. Once I have time though I'm definitely planning to come back and revamp bombino at least in the way it works under the hood to clean up some of the code and make the config a single file like this.

Although I guess some people might not want to store all config including sensitive information into version control, it should be easy to add an option for that.

This is a fair point, though I'd assume that they would use env variables for anything too sensitive, and would probably have bombino.config.js itself as a default entry inside the certignore array so that it gets stripped out before packaging and none of that config would be exposed. It should be purely for developer context and not production.