Fixed: force option

phan-do commented

2018-09-09 16:54:53 +00:00

(Migrated from github.com)

The force -f option was not working with either the theme generation option -g nor the generator updating option -u. There were 2 reasons:

The condition ((__force!=1)) on lines 260 and 306 were missing $ in front of the __force. So the value of the variable __force was not being compared, meaning whether or not -f was specified, ERX or ERR would always be called and the theme files would not be updated.
The value of global variable __force was never being changed, despite an assignment in the case statement on line 46. After a long ass time wondering why, I found out that having the assignment __force=1 on the same line as the expression, for some reason, doesn't change the value of __force.

After fixing these issues, the force option works with both -g and -u!

The force `-f` option was not working with either the theme generation option `-g` nor the generator updating option `-u`. There were 2 reasons: 1) The condition `((__force!=1))` on lines 260 and 306 were missing `$` in front of the `__force`. So the value of the variable `__force` was not being compared, meaning whether or not `-f` was specified, `ERX` or `ERR` would always be called and the theme files would not be updated. 2) The value of global variable `__force` was never being changed, despite an assignment in the case statement on line 46. After a long ass time wondering why, I found out that having the assignment `__force=1` on the same line as the expression, for some reason, doesn't change the value of `__force`. After fixing these issues, the force option works with both `-g` and `-u`!

👍 1

budRich commented

2018-09-09 17:10:14 +00:00

(Migrated from github.com)

Great. I hadn't noticed this issue myself, and made that refactoring (including the __ naming convention for global vars) a bit rushed.

@bl0nd , i am very grateful for this PR, i think it's awesome when people just try my stuff, even better when i get an issue, but a pull request! Amazing. I will test this quickly and if everything works (which i know it will), i will merge and create a new release today!

Since you have been digging in the code, is there anything you think should be changed, improved, added removed or such?

Great. I hadn't noticed this issue myself, and made that refactoring (including the __ naming convention for global vars) a bit rushed. @bl0nd , i am very grateful for this PR, i think it's awesome when people just try my stuff, even better when i get an issue, but a pull request! Amazing. I will test this quickly and if everything works (which i know it will), i will merge and create a new release today! Since you have been digging in the code, is there anything you think should be changed, improved, added removed or such?

budRich commented

2018-09-09 17:14:15 +00:00

(Migrated from github.com)

also strange errors.. ((__force==1)) should be valid (maybe this underscore business is breaking stuff, interesting), and the case thing, i wonder if it can't be written __force=1 && shift ;; ..

budRich commented

2018-09-09 18:40:09 +00:00

(Migrated from github.com)

i took a look at the option parsing part and fixed the force issue, and made the whole thing a bit more easy to manage (can be nice if the list of options grows). #7

according to my tests: ((__var==1)) is valid, and the (($__var==1)) fixes is not needed. This is one of the benefits with arithmetic tests, even better when there are arrays, we can write ((array[key]==1)) or ((${array[key]}==1)).

i took a look at the option parsing part and fixed the force issue, and made the whole thing a bit more easy to manage (can be nice if the list of options grows). #7 according to my tests: ((__var==1)) is valid, and the (($__var==1)) fixes is not needed. This is one of the benefits with arithmetic tests, even better when there are arrays, we can write ((array[key]==1)) or ((${array[key]}==1)).

phan-do commented

2018-09-09 18:54:55 +00:00

(Migrated from github.com)

Thanks a lot for fixing the issue! Bash isn't my primary language so I'm definitely gonna learn about those arithmetic tests, they seem really useful!

And I absolutely love all of your stuff man! I've learned a ton about i3 and bash from your videos. In fact, I was watching a video of yours a while back about better window switching or something and I saw a Todo program on the side and since you never talked about it I went ahead and wrote a copy-cat just cause it looked so cool.

Anyways, right now, the only thing I want to figure out is how to simplify the templates. Currently, every time I update a config file (say polybar's), I also have to update polybar's _mondo-template, otherwise when the former gets overwritten when I apply a theme, it'll be outdated by an edit. This updating gets a little tedious, especially when you got 4 or 5 other generators.

I thought it'd be a good idea to maybe have __mondo-template contain only lines from the original config that require substitution, so like for polybar:

~/.config/mondo/generators/polybar/_mondo-template

background = %%activebg%%
background-alt = %%inactivebg%%
foreground = %%activefg%%
foreground-alt = %%inactivefg%%
primary = %%blue%%
secondary = %%yellow%%
alert = %%red%%

That way, whenever we update the original polybar config, we don't need to update the polybar _mondo-template. And to apply the theme, we'd just need to update the original config with the generated theme files instead of overwriting the entire original config. The only problem is I don't know how to do that just yet. I think diff could help but the real problem is how to replace the appropriate lines in the original config file.

I saw that your mondo-contrib repo was structured like this so I have no idea if this is already a feature? If it is, it hasn't worked for me so far (though maybe I'm just being dumb and messing up) so thought I'd just bring it up.

Thanks a lot for fixing the issue! Bash isn't my primary language so I'm definitely gonna learn about those arithmetic tests, they seem really useful! And I absolutely love all of your stuff man! I've learned a ton about i3 and bash from your videos. In fact, I was watching a video of yours a while back about better window switching or something and I saw a Todo program on the side and since you never talked about it I went ahead and wrote a copy-cat just cause it looked so cool. Anyways, right now, the only thing I want to figure out is how to simplify the templates. Currently, every time I update a config file (say polybar's), I also have to update polybar's _mondo-template, otherwise when the former gets overwritten when I apply a theme, it'll be outdated by an edit. This updating gets a little tedious, especially when you got 4 or 5 other generators. I thought it'd be a good idea to maybe have __mondo-template contain only lines from the original config that require substitution, so like for polybar: ``` ~/.config/mondo/generators/polybar/_mondo-template background = %%activebg%% background-alt = %%inactivebg%% foreground = %%activefg%% foreground-alt = %%inactivefg%% primary = %%blue%% secondary = %%yellow%% alert = %%red%% ``` That way, whenever we update the original polybar config, we don't need to update the polybar _mondo-template. And to apply the theme, we'd just need to update the original config with the generated theme files instead of overwriting the entire original config. The only problem is I don't know how to do that just yet. I think `diff` could help but the real problem is how to replace the appropriate lines in the original config file. I saw that your mondo-contrib repo was structured like this so I have no idea if this is already a feature? If it is, it hasn't worked for me so far (though maybe I'm just being dumb and messing up) so thought I'd just bring it up.

budRich commented

2018-09-09 19:29:27 +00:00

(Migrated from github.com)

the todo thing, is a Sublime package called "PlainNotes" there is also "PlainTasks" by the same author that is more focused on todos. I haven't talked about it in the videos, since it is sublime, but now with my new sublime playlist i might make a short video about it, its great stuff.

Let's continue the discussion regarding "template injection" in #8. i have thought a bit about something similar, and it would be a cool feature, and i don't think it's hard to implement, but it might have undesired side effects..

the todo thing, is a Sublime package called "PlainNotes" there is also "PlainTasks" by the same author that is more focused on todos. I haven't talked about it in the videos, since it is sublime, but now with my new sublime playlist i might make a short video about it, its great stuff. Let's continue the discussion regarding "template injection" in #8. i have thought a bit about something similar, and it would be a cool feature, and i don't think it's hard to implement, but it might have undesired side effects..

Pull request closed

Please reopen this pull request to perform a merge.

Rows
Columns

Fixed: force option #6

Pull request closed