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

WIP DSL: add :glob key to expand fileglobs #5043

Closed
wants to merge 1 commit into from

Conversation

rolandwalker
Copy link
Contributor

This is a very rough/quick implementation, and is intended mostly for discussion,
as this is the DSL proposal in #4688 which has no associated PR.

Problems here include:

  1. This was implemented alongside :target in artifact/symlinked.rb, which also
    contains installation logic for artifacts. A smarter implementation would improve
    separation of concerns by moving the logic for interpreting both :glob and :target
    keys into dsl.rb (or some class called from dsl.rb)

  2. Only artifact sources such as link can be expanded. Similar functionality would
    also be useful (for example) in uninstall :files.

  3. This interface seems clunky. It might be cleaner to have an expand method in
    the DSL for wrapping filenames:

      link expand('*/VLC.app')

    The expand method would actually call a constructor, because the action of expansion
    must be deferred until install-time. However, I think Expander.new('*') is needlessly
    verbose for Cask authors, and not necessarily more self-documenting.

    • The expand interface might also help address problem 2 above.

References: Homebrew/homebrew-cask-versions#293 (comment)

@jawshooah
Copy link
Contributor

Closing for lack of interest, and because the implementation is vastly out of date at this point.

@jawshooah jawshooah closed this Oct 17, 2016
@miccal miccal removed core Issue with Homebrew itself rather than with a specific cask. enhancement labels Dec 23, 2016
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants