More flexibility with config-blocks (i.e. more than just the two)
Opened this issue · 4 comments
I've been implementing this at https://github.com/D-Vaillant/j4-make-config/tree/class but I wanted to have a forum for design feedback. For the sake of efficiency I've chosen to force all of the config block markers start with # $i3-
(fixed in the PREFIX
variable) so that we can pluck them out by just checking for PREFIX in line
.
Some thoughts:
-
Do we want to require that the prefix starts with a
#
or is it better to prepend it automatically? -
Are we concerned about more than one
#
s? -
If we want to use
$i3-
as the config-block marker prefix we might not want to erase it if there's no corresponding config-block, on the off-chance that somebody out there has it has some title or something. That could probably be a configuration option. Very low priority.
-
Maybe we check if the prefix starts with
#
to be explicit... so users can use### $i3-theme
if they want to... and is less dirty this way. If they don't include#
, they will learn really fast... Thanks to i3 config parser or we can add#
ourselves. Maybe it is better to keep# $i3-
hardcoded so we don't introduce silliness and/or prefix headaches... since I assume we'll have to keep specifying it in the optional parameters. -
See above. I think if we add a prefix, we use it... so
#
would not matter. Prefix matters. -
We probably shouldn't erase them in first place... meaning we would see
# $i3-theme-window
in the config too... right on top of the concatenated code. Optionally, we could remove$
to see# i3-theme-window
instead.- I think we won't have problem with
line.startswith(prefix)
instead ofprefix in line
... and I can't see how users can have some titles or something that makes them write# $i3-
. Commenting out aliases and exec would give usset $i3
--># set $i3
... andexec $i3
--># exec $i3
so maybe we're good here.
- I think we won't have problem with
My thoughts.
EDIT: P.S. I looked at your class
branch.
class: ./j4-make-config --list-themes
Traceback (most recent call last):
File "./j4-make-config", line 320, in <module>
printThemeList()
NameError: name 'printThemeList' is not defined
Oh, the class
branch is a nightmare factory right now. The commits I just pushed make it work without immediately breaking but I haven't even really tested it with non-standard block names.
The ability to let people add multiple #'s is a good idea. I was thinking of using split('#')
. Something like:
def foo(name="\t\t###!### $my-prefixfooblockcfgname"):
# we can use a try/except block to filter out lines without the prefix
head, blockconfigname = name.split(PREFIX) # =["\t\t###!###", "fooblockcfgname"]
# we can throw in an try/except block here to check if we don't have a '#'
whitespace, tail = head.split('#') # =["\t\t", a mess of strings]
tail = '#' + ''.join(tail)
Right now it doesn't distinguish between the leading whitespace and any other leading character, so additional #'s end up commenting out the block. And this might break if someone uses PREFIX more than once in a line (which they shouldn't, but when it happens it just moves on.) It shouldn't be a difficult change for the # thing but it's fairly low priority for me right now.
I like the idea of preserving the block headers so I just added them back in. This has the upside of preserving any line that looks like a block header but doesn't have any blocks associated with it.
I changed the prefix in line
to line.startswith(prefix)
for j4Config. Makes more sense, cleaner.
I'm busy right now with RL and other projects but I'm trying to have a look in the next days.