JakobGM / astrality

Astrality - A modular and dynamic configuration file manager
https://astrality.readthedocs.io
MIT License
98 stars 3 forks source link

Achieve stable configuration syntax for v1.0 #9

Closed JakobGM closed 6 years ago

JakobGM commented 6 years ago

I have done a huge refactoring of the code base. The goal has been to extract responsibilities from Module and ModuleManager, and implement them in a greater number of small classes. The result is two new modules, astrality.actions and astrality.requirements.

Astrality's configuration is now parsed and instantiated as small classes. Let's say you have the following configuration file:

module/A:
    requires:
        - shell: some/script.sh

    on_startup:
        import_context:
            - ... options ...

    on_event:
        compile:
            - ...options...
        run:
            - ... options ...

This results in a object hierarchy somewhat along the lines of:

module.ModuleManager:
    - module.Module(module/A):
        - requirments.Requirement(shell=some/script.sh)

        - actions.ActionBlock(on_startup):
            - ImportContextAction(... options ...)

        - actions.ActionBlock(on_event):
            - CompileAction(...options...)
            - RunAction(... options ...)

The goal is that if you for instance would like to change the behaviour of the run action, you should be able to write code exclusively in astrality.actions.RunAction instead of bouncing between astrality.module.Module and astrality.module.ModuleManager.

I have also changed most of the options that have previously been list of strings to dictionaries, as that makes it possible to add extra options in the future without breaking end-user's configuration files. We can just add a new key with a default value instead!

This is the first step towards releasing Astrality v.1.0, which shouldn't be too far in the future. I have basically made all backwards incompatible changes in this PR.

I would like your feedback on this @sshashank124, if you find the time. The code diff is far too extensive in order to read, so don't even bother :P But you could perhaps read over the CHANGELOG and see if you agree with the changes? If the option keys make sense, and so on?

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 301


Changes Missing Coverage Covered Lines Changed/Added Lines %
astrality/actions.py 181 182 99.45%
<!-- Total: 735 736 99.86% -->
Files with Coverage Reduction New Missed Lines %
astrality/module.py 3 96.4%
<!-- Total: 3 -->
Totals Coverage Status
Change from base Build 256: 0.3%
Covered Lines: 3086
Relevant Lines: 3165

💛 - Coveralls
JakobGM commented 6 years ago

Ping, @sshashank124.

Just added support for compiling entire directories recursively.

Next on the list:

compile:
    source: '/path/to/templates'
    target: '/path/to/config/directory'
    extension: '.template'

Which should do the following: find all recursive files with file extension ".template" and compile those to target files without* the extension.

Just wondering if we should only support extensions, or if we would want something along the lines of:

compile:
    source: '/path/to/templates'
    target: '/path/to/config/directory'
    regex: '(.+)\.template'

regex could also be named rename.

Using the regex capture group as the compilation target filename. That would require users to understand constructs such as escaping, identifiers, and group captures, so I guess the usability gain is negated by the increased complexity. I'm a bit blind here, since I'm quite used to using regexes daily through search and replace in vim :stuck_out_tongue:

Anyway, I think the extension removal case is by far the most common usecase (for myself at least).

sshashank124 commented 6 years ago

All the changes look great! I like the idea of compiling directories recursively while maintaining file structure. However, I think the extension option might be too tailored for a specific use-case. Personally, I prepend t. to my templates to allow vim to determine the filetype better in some cases without having to manually specify # vim:ft=... for when the extension for a template is .template. Also, I'm not a fan of the regex solution since it might not be the most intuitive for some people. I'm thinking that if there is a dedicated directory for a set of files to compile, it shouldn't be necessary to name the template files with a .template extension in the first place. But, it'd be nice to have the rename option without cutting it out completely.

All in all, the best course would be to compile files with the same filename unless a rename/regex field is specified where the regex option's value corresponds to the following in a typical regex expression

's/abc/def/g'
   ^^^^^^^

but maybe a little more straightforward. Either way, I don't think we should specifically limit the option to only work on extensions using extension

JakobGM commented 6 years ago

Thanks for the feedback!

Let's merge this PR into master, since there seems to be no major objections to the configuration syntax already implemented. I will create a seperate issue for filters, renames, and file extensions, and then we can continue the discussion there :+1: