Different preludes for different folders

[gelisam]

What is the purpose of the user prelude? Is it meant to accumulate the multitude of helper functions which our users have built over the years, or to help users cope with commands which would otherwise take more than one line?

I like to use it for the latter, and as a consequence I have many versions of my user prelude, because I don't like when it becomes too big and because the functions I write for one project are rarely useful in the next project.

To cater to this use case, I propose to follow git's model of user preferences: look into the parent directories for a .git/config file (in our case .hawk/prelude.hs), and fall back to ~/.gitconfig (in our case ~/.hawk/prelude.hs) if none is found.

What do you think?

[melrief]

I also think the user should be able to use different preludes for different purposes. I see only one small drawback in your idea: the user can use only one prelude for each directory (the fall back or the one inside ./.hawk/prelude.hs). I don't know if this can be a problem. Another approach would be to have an option to specify the prelude directory (hawk -p ./.hawk does exactly what you were saying). In this case the user could select the right prelude in any moment. This idea is more verbose than yours.

[gelisam]

Maybe there could be a flag to explicitly specify the prelude to use (hmm, I thought we already had that, apparently not), and if nothing is specified it would default to searching parent directories?

Another idea: instead of defaulting to ~/.hawk/prelude.hs, maybe we could combine it with the one specified on the command-line or found in a parent directory?

[melrief]

I'm don't understand the second idea, can you elaborate it (in particular the idea of combine)?

The first one is ok for me.

[gelisam]

By "combining" prelude files, I mean that the user expression would have access to function definitions from both files. This way, users can put frequently used custom functions in their ~/.hawk/prelude.hs, and their project-specific custom functions in their project-specific prelude files.

[melrief]

For this I need a new option. Before I code it I have two questions:

1 - do you mind if I call the directory config-dir instead of prelude-dir and I use -c/--config-directory as command line argument? In future this directory will contain both the user prelude and the configuration.
2 - more important: the -c argument should be checked. My idea is that there are only two possibilities:

• the directory exists, all files are readable and <config-dir>/cache is writable: hawk uses that directory as config-dir
• no files with the given name exist in the system and the parent directory is writable: hawk will create a standard config-dir with a standard prelude.hs inside and print to stderr a warning that there is no directory and it created it.

in all the other cases Hawk should give error. That means that we need IO to check the filesystem. If I understand correctly how your optionParser.hs works, this is not implemented. Can you tell me if my idea is correct?
To add this control I have to:

• add a HawkOption for this, of course
• add a new value in Control.Monad.Trans.OptionParser.hs like

file :: OptionType
file = Setting "file"

• add a consumeFile that is a bit different from the other consume actions because it also takes in input a checker functions that checks that the state of the given file is correct:

consumeFile :: MonadIO m => (String -> ErrorT String m Bool) -> OptionConsumer m String
consumeFile checker (Just f) = ...
consumeFile checker Nothing = error "please use consumeNullable to consume nullable options"

• change userPrelude of ExprSpec into userConfigDirectory so that it can hold the given option
• use consumeFile to produce ExprSpec in exprSpec. The type of exprSpec becomes exprSpec :: (Functor m, MonadIO m) => OptionParserT HawkOption m ExprSpec
• change the type of parseArgs to (MonadIO m) => [String] -> UncertainT m HawkSpec and runOptionParserWith in the same way.
• from now it should be simple: I just need to change the default directory into the given directory or default.

... and that's it, right? Is it correct? I have to admit that your library for working with arg/option/spec is still new to me :-).

[melrief]

Created the branch different_preludes to add this feature.

[gelisam]

do you mind if I call the directory config-dir instead of prelude-dir

Sure, call it whatever you want.

file = Setting "file"

Since the option is called config-dir, shouldn't this be a folder instead of a file?

add a consumeFile that is a bit different from the other consume actions because it also takes in input a checker function

That's a good idea! When I implemented OptionParserT, I was thinking that there should be a file OptionType which would make sure that the file exists. But I didn't realize that there are two kinds of file arguments, readable files and writable files! For folders, there are even more combinations. Making consumeFile parameterizable solves this issue nicely.

consumeFile :: MonadIO m => (String -> ErrorT String m Bool) -> OptionConsumer m String

Why String -> ErrorT String m Bool instead of FilePath -> UncertainT m Bool?

and runOptionParserWith in the same way

Yes, you will need to change a few type constraints from Monad to MonadIO, but why do you also need to change runOptionParserWith? It already works with all monads, whether they satisfy MonadIO or not.

from now it should be simple: I just need to change the default directory into the given directory or default.

Actually, I think this is the part at which it starts to become complicated :) There are probably many places which use getConfigDir directly or indirectly, and they might not already have an ExprSpec handy. The only place in which we should use getConfigDir is in getEvalContext, but I think there are also many other places which duplicate part of the work done by getEvalContext, and you will either need to fix that or to leave a second copy of getConfigDir in place with the old behaviour.

. and that's it, right? Is it correct? I have to admit that your library for working with arg/option/spec is still new to me :-)

It's new for everyone :) But from what you wrote, you seem to understand all of it already.

[melrief]

I have implemented this in the different_preludes branch with the commit 5d52e86. It's working and all the tests are passing.

I would like to continue to work on this because I need to add the support for projects configurations and I also would like to clean the System.Console.Hawk.Config module. After your refactoring, the only important function that we are using of that module is recompileConfig. The recompileConfigIfNeeded function is deprecated because now getEvalContext is in charge of checking the cache. So recompileConfigIfNeeded can be deleted.

I also have doubts on which module should handle the creation of a default config directory if this doesn't exist. Currently there is a function System.Control.Hawk.Context.createDefaultIfDoesntExist that creates the default context (configuration directory + default prelude.hs) if it doesn't exist and is used by getEvalContext. But I'm not sure this is the right place for this function. Maybe it is better to do it inside the consumeFilePath function, where hawk checks that the configuration directory exist and has the right permissions. This function takes an "action" of type (MonadIO m) => FilePath -> UncertainT m () that is currently used to fail with error if the config directory is not in a good state (see above). We could use that "action" to create the default directory if it doesn't exist and use getEvalContext only for checking the cache...

By the way, it's nice to be able to write:

> hawk -c /tmp/fakehawkconf 1
warning: directory '/tmp/hawk2' doesn't exist, creating a default one
1

[gelisam]

Yes, I think consumeFilePath is a good spot for that, and I agree that we need to remove recompileIfNeeded. We should also remove the code which creates cache/modules and cache/extensions, but I think there might still be some code using those instead of EvalContext.

Meanwhile, I am working on spreading the use of Uncertain instead of Either.

[gelisam]

Should we create migration code which removes the modules and extensions files which 1.0 has created? I assume that most people will never clean their .hawk/cache.

[gelisam]

We should have a cache/version file. Otherwise read will crash when we add md5 to the key and the cached cache/evalContext still uses size and timestamp.

[gelisam]

The logic for versioning could be quite simple: if there is no cache/version or if it contains the wrong value, remove the entire cache folder and create it anew.

[melrief]

I have implemented the projects custom context in the commit a5ff205. Now if the -c option is not specified, Hawk seaches for .hawk directories from the current working dir to the root and if it doesn't find it it sets the directory to ~/.hawk.

I added a few functions to System.Console.Hawk.Context but their position is not definitive. I will open a issue to discuss about the module organization because I think we should clarify what prelude/config/context/specs/etc are (edit: done #85).

I don't use consumeFilePath anymore to get the context directory, but I'm not removing it because it can be useful for something else, like the input file.

I'm not closing this because I can't make tests working..the problem is that I should create temporary directories and remove them. I will do it.

One last point: I've added the liftUncertain function to Control.Monad.Trans.OptionParsers to lift Uncertain functions:

liftUncertain = OptionParserT . lift . lift

I think it is ok but can you confirm it for me?

I just noticed that the time to run hawk 1 is now around 0.4s. Are we ok with this?

[gelisam]

the problem is that I should create temporary directories and remove them.

Control.Monad.Trans.State/Persistent contains example tests which create and cleanup files.

liftUncertain = OptionParserT . lift . lift
I think it is ok but can you confirm it for me?

Looks good!

I just noticed that the time to run hawk 1 is now around 0.4s. Are we ok with this?

It's slower than before? By how much? I think that since the refactoring is incomplete, we are probably doing a lot of work twice, so if it was taking 0.2s before, that gives us a good idea why.

[gelisam]

I measure 0.24s on master, 0.24s on develop, and on 0.24s different_preludes. All measurements were taken with a cached prelude (I couldn't measure with an uncached prelude because of #87).

So if it's a regression, it's been there since before 1.0.

[melrief]

As requested in #86 I merged different_preludes in develop.

[melrief]

I have removed the different_preludes branch and I'm closing this. For future work on this we will open another issue.