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

Factor out and export defaultCommands #959

Merged
merged 1 commit into from Nov 15, 2022
Merged

Conversation

ccntrq
Copy link
Contributor

@ccntrq ccntrq commented Nov 2, 2022

This reduces the amount of copy-pasting required to add custom sub-commands to the hakyll cli.

For my personal hakyll-created website, I implemented a myHakyll function that uses a custom myOptionsParser and myCommandParser to allow for custom command line options and commands:

data MyCommand = HakyllCommand Command
               | MyCustomCommand

data MyOptions = MyOptions { verbosity :: Bool, myOptCommand :: MyCommand }

myHakyll :: Configuration -> Rules a -> IO ()
myHakyll conf rules = do
  args <- myParser conf
  case myOptCommand args of
    HakyllCommand com
      -> hakyllWithArgs conf (Options (verbosity args) com) rules
    GenerateWeeklyChallenges -> generateNewChallengeFiles

While I want to add some custom commands, I also would like to keep all the base commands provided by hakyll. With the currenthakyll v4.15.1.1 I had to copy-paste the command definitions from Hakyll.Main.commandParser for my own myCommandParser to achieve this. (See this commit for the full implementation)

With the addition of defaultCommands to the public interface of hakyll this copying and pasting is no longer necessary. Now I can import defaultCommands and fmap HakyllCommand over the second component (Parser Command) of the command tuples returned by defaultCommands and add my custom commands to the end. (See commit: Remove copy pasted code from hakyll for an example)
Additionally, this removes the need to manually keep the copy-pasted list of commands up to date with the current hakyll version whenever an update is applied, as we now get the latest set of commands from hakyll automatically.

It would be awesome if this could be added to the public API. I'm happy to keep working on this if any changes are required before a merge is possible. Let me know what you think.

Kind regards,
Alexander Pankoff

@Minoru
Copy link
Collaborator

Minoru commented Nov 15, 2022

Sorry it took me two weeks to take a look! The code looks good, let's see if it passes CI :)

@Minoru Minoru merged commit b4c224c into jaspervdj:master Nov 15, 2022
@ccntrq
Copy link
Contributor Author

ccntrq commented Nov 17, 2022

Great. Thanks for merging this.

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

Successfully merging this pull request may close these issues.

None yet

2 participants