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

commands: filter out dotfiles from output #3334

Merged
merged 2 commits into from Oct 20, 2017

Conversation

Projects
None yet
4 participants
@DomT4
Contributor

DomT4 commented Oct 18, 2017

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully run brew tests with your changes locally?

Noticed in this that DS_Store was getting chucked in with the commands list πŸ€·β€β™‚οΈ.

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Oct 18, 2017

Contributor

not all dot files?

Contributor

ilovezfs commented Oct 18, 2017

not all dot files?

@DomT4

This comment has been minimized.

Show comment
Hide comment
@DomT4

DomT4 Oct 18, 2017

Contributor

I can if you want? Just thought this was the only one that seemed likely to end up inside a cmd folder unless someone is doing something a little strange with their tap/etc.

Contributor

DomT4 commented Oct 18, 2017

I can if you want? Just thought this was the only one that seemed likely to end up inside a cmd folder unless someone is doing something a little strange with their tap/etc.

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Oct 18, 2017

Contributor

It will currently list e.g. .info.rb.swp so I would do all dot files, yeah

Contributor

ilovezfs commented Oct 18, 2017

It will currently list e.g. .info.rb.swp so I would do all dot files, yeah

@DomT4

This comment has been minimized.

Show comment
Hide comment
@DomT4

DomT4 Oct 18, 2017

Contributor

Alright. One sec.

Contributor

DomT4 commented Oct 18, 2017

Alright. One sec.

@DomT4 DomT4 changed the title from commands: filter out DS_Store files from output to commands: filter out dotfiles from output Oct 18, 2017

@reitermarkus

This comment has been minimized.

Show comment
Hide comment
@reitermarkus

reitermarkus Oct 18, 2017

Member

How about

  Pathname.glob(directory/"*")
          .select(&:file?)
          .map { |f| f.basename.to_s.sub(/\.(?:rb|sh)$/, "") }

?

Member

reitermarkus commented Oct 18, 2017

How about

  Pathname.glob(directory/"*")
          .select(&:file?)
          .map { |f| f.basename.to_s.sub(/\.(?:rb|sh)$/, "") }

?

@DomT4

This comment has been minimized.

Show comment
Hide comment
@DomT4

DomT4 Oct 18, 2017

Contributor

I don't have a major objection if people prefer that format. Whatever folks are happy with. I am not a huge fan of chaining together across multiple lines like that but it's become a common enough thing I can't really start complaining about it now πŸ˜„.

Contributor

DomT4 commented Oct 18, 2017

I don't have a major objection if people prefer that format. Whatever folks are happy with. I am not a huge fan of chaining together across multiple lines like that but it's become a common enough thing I can't really start complaining about it now πŸ˜„.

@reitermarkus

This comment has been minimized.

Show comment
Hide comment
@reitermarkus

reitermarkus Oct 18, 2017

Member

I find it easier to follow than a next inside an if inside an each_with_object. πŸ˜‰

Member

reitermarkus commented Oct 18, 2017

I find it easier to follow than a next inside an if inside an each_with_object. πŸ˜‰

@DomT4

This comment has been minimized.

Show comment
Hide comment
@DomT4

DomT4 Oct 18, 2017

Contributor

@ilovezfs Any objections? Keen to avoid getting in the middle of a maintainer disagreement over style. I don't miss those πŸ˜….

Contributor

DomT4 commented Oct 18, 2017

@ilovezfs Any objections? Keen to avoid getting in the middle of a maintainer disagreement over style. I don't miss those πŸ˜….

commands: tweak find_internal_commands.
Use a more typical Ruby style.
@MikeMcQuaid

This comment has been minimized.

Show comment
Hide comment
@MikeMcQuaid
Member

MikeMcQuaid commented Oct 20, 2017

Thanks @DomT4 and @reitermarkus!

@MikeMcQuaid MikeMcQuaid merged commit a2374cb into Homebrew:master Oct 20, 2017

3 checks passed

codecov/patch 100% of diff hit (target 67.91%)
Details
codecov/project Absolute coverage decreased by -0.23% but relative coverage increased by +32.08% compared to 270b752
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@DomT4 DomT4 deleted the DomT4:ds_store_is_not_a_command branch Oct 20, 2017

@DomT4

This comment has been minimized.

Show comment
Hide comment
@DomT4

DomT4 Oct 20, 2017

Contributor

Thanks for merging Mike. Was trying to give ILZ a little bit to see if he had any objections, but I'm assuming we're good.

Contributor

DomT4 commented Oct 20, 2017

Thanks for merging Mike. Was trying to give ILZ a little bit to see if he had any objections, but I'm assuming we're good.

@ilovezfs

This comment has been minimized.

Show comment
Hide comment
@ilovezfs

ilovezfs Oct 21, 2017

Contributor

LGTM. Thanks, Mr. T

Contributor

ilovezfs commented Oct 21, 2017

LGTM. Thanks, Mr. T

@Homebrew Homebrew locked and limited conversation to collaborators May 4, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.