Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Allow passing a list of files to mix credo #4

Closed
BachirC opened this issue Mar 6, 2017 · 18 comments
Closed

Proposal: Allow passing a list of files to mix credo #4

BachirC opened this issue Mar 6, 2017 · 18 comments

Comments

@BachirC
Copy link

BachirC commented Mar 6, 2017

While trying to add credo analysis in my pre-commit git hook on staged files, I noticed that actually only the first file of the list was anaylzed. (Credo : 0.6.1, Elixir : 1.4.1, default .credo.exs)

So being able to do something like : mix credo [options] file1.exs file2.ex file3.ex would be really nice imo ! :)

@rrrene rrrene transferred this issue from rrrene/credo Dec 8, 2019
@ffloyd
Copy link

ffloyd commented Jan 10, 2020

+1

@joskar
Copy link

joskar commented Jan 16, 2020

Is someone working on this to make this happen?

@rrrene
Copy link
Owner

rrrene commented Jan 17, 2020

Credo 1.2.0-rc3 now features --files-included/--files-excluded switches.

Both switches accept files, globs and can be given multiple times, potentially combining both.

mix credo --files-included file1.exs
mix credo --files-included file*.exs
mix credo --files-included file1.exs --files-included file2.ex --files-included file3.ex

We'll see if we can implement this in the proposed way, but for now this is what we have 👍

@NorthIsUp
Copy link

is mix credo --files-included file1.exs file2.ex file3.ex also an option? When trying to pass through other build tools getting the --files-included to interlace with the files can be difficult or impossible to script.

@rrrene
Copy link
Owner

rrrene commented Feb 25, 2020

is mix credo --files-included file1.exs file2.ex file3.ex also an option?

Unfortunately, no. I also would not consider that a viable candidate and do not know of any tool accepting a cumulative option like that. How you would you even parse that? (honest question?)

@prem-prakash
Copy link

Being able to pass a list of files would be really useful for pre-commit hooks.

@rrrene
Copy link
Owner

rrrene commented Jul 24, 2021

@prem-prakash That is possible, please see this comment.

mix credo --files-included file1.exs --files-included file2.ex --files-included file3.ex

@rrrene rrrene closed this as completed Jul 24, 2021
@prem-prakash
Copy link

See this comment #4 (comment)

@rrrene
Copy link
Owner

rrrene commented Jul 25, 2021

See this comment #4 (comment)

I am sorry, but I already replied to that comment, urging the author to state how that should work.

I would pose the same question to you: How would the OptionParser be able to parse this unambiguously?

You could also point out tools that work this way, so we could look up how they do it. 👍

@prem-prakash
Copy link

Hi Rene, sorry for insisting. In fact, OptionParser is a bit limited in that it cannot deal with a list of values for an option in another way than repeating the option prefix. Thanks for your attention.

@rrrene
Copy link
Owner

rrrene commented Jul 25, 2021

I did not want to discourage you. ✌️

My understanding is that it is more or less trivial to control the files in a scripting environment like BASH:

function run_credo {
    cmd=(mix credo)
    for file in "$@"; do
        cmd+=("--files-included" "$file")
    done

    command "${cmd[@]}"
}

run_credo `ls lib/*.ex`

I am just curious how we could provide a more convenient version that is as precise as the culmulative option solution while making it more approachable for non-scripting experts.

@alex9spiridonov
Copy link

See this comment #4 (comment)

I am sorry, but I already replied to that comment, urging the author to state how that should work.

I would pose the same question to you: How would the OptionParser be able to parse this unambiguously?

You could also point out tools that work this way, so we could look up how they do it. 👍

@rrrene
you can see rubocop for ruby
https://docs.rubocop.org/rubocop/usage/basic_usage.html
you can pass rubocop a list of files and directories to check

@rrrene
Copy link
Owner

rrrene commented Aug 29, 2022

@alex9spiridonov Rubocop does not work in the discussed fashion. Please re-read the discussion above 👍

@prem-prakash
Copy link

prem-prakash commented Aug 29, 2022

@rrrene yes it does

Screen Shot 2022-08-29 at 16 40 38

@rrrene
Copy link
Owner

rrrene commented Aug 30, 2022

@prem-prakash there is no need to be passive agressive here.

The option discussed above was to have a single option that receives the space-separated list of files (see the linked and quoted comment as well as the discussion re: OptionParser).

You are posting a screenshot of a program that receives a list of files as arguments. I'm sorry, but that is not the same thing.

@prem-prakash
Copy link

@rrrene sorry, it was not my intention to sound passive-aggressive.
This is just a very wanted feature to simplify credo use especially for big projects.

@alex9spiridonov
Copy link

alex9spiridonov commented Sep 8, 2022

@prem-prakash maybe it will be useful for you.
In our company we wrote this CI script to check only modified files.
This script builds string like --files-included changed_file1.exs --files-included changed_file2.ex --files-included changed_file3.ex and pass it to mix credo.

- CHANGED_FILES=$(git diff-tree -r --no-commit-id --name-only --merge-base --diff-filter=d origin/master HEAD)
- FILES_INCLUDED=""
- for f in $(echo $CHANGED_FILES | tr ";" "\n") ; do FILES_INCLUDED="$FILES_INCLUDED --files-included $f" ; done
- if [ ! -z "$CHANGED_FILES" ] ; then echo $FILES_INCLUDED | xargs sudo -E mix credo --strict ; fi

@saveman71
Copy link

Note to future readers: credo 1.7 now supports this out of the box

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

No branches or pull requests

8 participants