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

Modularize? #2

Open
Ambrevar opened this issue Jan 19, 2018 · 7 comments
Open

Modularize? #2

Ambrevar opened this issue Jan 19, 2018 · 7 comments

Comments

@Ambrevar
Copy link

Many of the your scripts contain a lot useful stuff. But much of it is packed together in a way that makes it hard to re-arrange their composition from the commandline.

One of Demlo's strength is specifically to trigger which "component" to execute at runtime.
A simple -s foo or -r bar is enough. When several features are within one scripts, that's much harder.

With that, it's also possible to have "metascripts" that enable a set of scripts.

Take the following example:

$ cat 10-foo.lua
if inhibit_foo then return end

-- Rest of the code here.
-- Do only one thing.


$ cat 11-bar.lua
if inhibit_bar then return end

-- ...

$ cat 12...
...

$ cat 10--meta_all.lua
inhibit_foo=nil
inhibit_bar=nil
...

$ cat 10--meta_none.lua
inhibit_foo=true
inhibit_bar=true
...

$ cat 10--meta_foo_and_some_other_scripts.lua
inhibit_foo=nil
inhibit_bar=true
...

$ cat demlorc
Scripts = {'10-foo', '11-bar', ...}

Then the user can simply select the right metascript with all the normal scripts on and it will do the right thing.

This is similar to what you've done with the globals script, except that it is easier to tweak from commandline: auto-completion can list both the scripts and the metascripts. No need for extra variables nor extra documentation. No need for cumbersome -pre scripts either.

What do you think?

@fictionic
Copy link
Owner

This is a good idea. Using -pre for everything is indeed quite cumbersome. The only downside I can think of is that it would create a LOT of new scripts to populate the folder.

So are you suggesting that I do away with the globals script entirely? Or simply that I remove its responsibility for reading in all the on-the-fly settings and transfer them to metascripts?

Could you provide a more in-depth example for how this would work? Where are some place you would put inhibit or enable variable checks in my scripts, and what metascripts would you use for them?

Thanks for all your input!

@Ambrevar
Copy link
Author

I've been thinking more about this and I think the concept of "meta" script would add needless complexity.

What about the following?

  • Make your scripts more modular, for instance move the "tag extraction" to a separate script.

  • Remove the settings.tag.extract_tags = true as it's no longer needed.

  • Both "meta" concept or your "settings" variable are indeed equivalent to a preselection of scripts (some of them setting global variables).
    There are a few ways to go about this:

    • Create some wrapper scripts around Demlo with the appropriate -s ... sequence.
    • Same with shell aliases.
    • More "demlo-ish": create several Demlo configs with the desired Scripts slices and start Demlo with DEMLORC=foorc demlo ....

@Ambrevar
Copy link
Author

The upside of using config files is that you can store them in music folders / libraries, thus making it easy to call Demlo with the right config (that is, script selection) contextually.

@fictionic
Copy link
Owner

I do admit that tag extraction should be in its own script; I just never got around to separating it. How do you propose to deal with the issue of fine-tuning how tags should be extracted? Currently it reads variable ftextr (coming up with an abbreviated name for force extract tags is tough...), which should be a comma-separated list of tag names, or the string "all"—the script then always uses the extracted value rather than any preexisting value.

As for the other options you suggest... well, they seem kind of "hacky" to me. Although I do think demlo should have a -c <configfile> argument. Or another name if you don't want to break backwards compatibility with downloading covers from the internet (a feature which I had forgotten existed until I checked just now for whether there already was a -c flag). Maybe -u for "use"? That's what vim does. Or turn -c into -cover.
Only because it would work more elegantly with shell aliases.

Though this is basically doing what my 'profile' scripts were trying to do. Each profile script would set up the rest of the scripts to carry out execution for a specific use case. Why not combine those with your idea of metascripts—just putting in more boolean variables to control execution of later scripts?

@Ambrevar
Copy link
Author

Only because it would work more elegantly with shell aliases.

Doesn't it work with env DEMLORC=foorc demlo ...?
The reason the config file is not a parameter is twofolds:

  • Current config values can be displayed in the help message.
  • It greatly simplifies the implementation.

Regarding the meta-scripts / profiles: I think sticking to "profiles" is simple enough and it works well as long as scripts are modular enough.

@fictionic
Copy link
Owner

Ah I forgot about env. Good point.

How does taking the config file from the command line complicate the implementation?

And ok good, let's stick with profiles.

@Ambrevar
Copy link
Author

Ambrevar commented Jan 29, 2018

How does taking the config file from the command line complicate the implementation?

It's a bit stupid, actually: the config and the commandline parameters tweak the same variables. Now which one has precedence?
Commandline parameters should, which means that if we go for a -c flag we must parse the flags a first time, load the config, then a second time without loading the config.
The problem is, the "flag" standard library in Go does not support double-parsing in its simplest form (correct me if I'm mistaken). I think I would need to copy the commandline parameters and define custom FlagSet variables.

https://golang.org/pkg/flag/#FlagSet

Probably not very hard, but if the env way works, do we really need a -c flag?

Regarding this issue: we can close it for now, but I'm still very interested to bring some of your stuff upstream.
Try to split your scripts into atomic bits: monolith are not very useful upstream, plus it will help me test and approve what can be integrated.

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

No branches or pull requests

2 participants