Ana06/get-changed-files

[Feature] Filter : include/exclude

Closed this issue · 6 comments

rnsc commented

Hello @Ana06 ,

I've looked at the code already and I see that the filtering implementation is leveraging regex to filter files.
I have a particular case that would require include a certain type of files but also exclude specific paths at the same time.
ie:

  • blah/test/something.yml
  • blah/not_at_test/something_else.yml
  • blah/another_folder/something.yml
  • blah/another_folder/something_not_a_yaml.j2

So I need to match all *.yml, but not *.yml that are in a test folder for example.
Now, while I undertsand I could make a convoluted regex to do that, I would also want to keep it simple.

Would you welcome a change where we change the filter by include and exclude both supporting regex, and add a bit more code logic?

I'd be willing to have a poke at it.

Thanks,

micalevisk commented

This would be pretty cool to have.

Also, I'd suggest you change the new RegExp(`/${filter}\\b`, 'g') bellow to new RegExp(filter, 'g') (or am I missing something?) -- with this one could use filters like \.(js|ts)$ (instead of *\.(js|ts)$) and another more flexible pattern

get-changed-files/src/main.ts

Line 79 in a2f6df8

const regex = new RegExp(`/${filter}\\b`, 'g')

And replace file.filename.match(regex) with regex.test(file.filename)

get-changed-files/src/main.ts

Line 81 in a2f6df8

const files = response.data.files.filter(file => file.filename.match(regex))

Ana06 commented

@rnsc I think that is a great idea! Thanks for submitting it the issue. You are more than welcome to send a PR 😉

Ana06 commented

@rmed19 I would like to know your opinion about this, since the current implementation is yours. 😄

agraul commented

What do you think about modeling the include/exclude filters after GitHub Actions' on.<push|pull_request>.paths? The options would be called paths and paths-ignore and accept lists of patterns instead of a string. It would make the change in #2 more complex, and I'm not sure if it's needed, but it might simplify workflow yaml files.

A use case that I can think of is that you only want to run an action when foo/bar.x is part of a commit and you also only want to run $test on foo/bar.

  • ❤️1
rnsc commented

What do you think about modeling the include/exclude filters after GitHub Actions' on.<push|pull_request>.paths? The options would be called paths and paths-ignore and accept lists of patterns instead of a string. It would make the change in #2 more complex, and I'm not sure if it's needed, but it might simplify workflow yaml files.

A use case that I can think of is that you only want to run an action when foo/bar.x is part of a commit and you also only want to run $test on foo/bar.

Hey, thank you for the feedback!
Not sure I'm seeing the use case of accepting list of regexes though. Can't that be solved with a "simple" logic operation in the regex?

agraul commented

My main motivation is to stay close to what already exists, the list of regexes is not something I came up with myself, it's GH's way™. However, it allows to have positive and a negative patterns grouped together:

with:
  paths:
    - 'sub-project/**'
    - '!sub-project/docs/**'

One aspect I didn't mention is that GH does not allow both paths and path-ignore at the same time, instead the syntax from this example is to be used (source).

Maybe it does not need to be this complex, the main advantage I see is that you could copy&paste the paths: list from the on: section to the with: section of this action.

  • 👍2