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

Blocklet's environment #279

Closed
vivien opened this issue Jan 2, 2018 · 9 comments
Closed

Blocklet's environment #279

vivien opened this issue Jan 2, 2018 · 9 comments

Comments

@vivien
Copy link
Owner

vivien commented Jan 2, 2018

@kb100 stated on 09d8db1:

I think we should get a discussion going about the best way to transfer block environmental variables to blocklets since this convention is going to be very hard to change once it is decided. My main objection to the $BLOCK_UPPERCASENAME convention is that it makes variable names long. My personal preference would be be for name to convert to $_name or $_NAME. Is there any way this would conflict with other things in the block's environment, or would this be safe?

Indeed the blocklet's environment is going to be the standard way to pass data and variables to a script. Any non-empty string not containing = is accepted by setenv(3), thus we could simply pass the environment as is. This means that the following blocklet:

[blocklet]
foo=bar
command=date +%R
interval=60
FOO=bar

would inject the $foo, $command, $interval, $FOO environment variables.

While lowerspace variables aren't really appreciated, they are totally valid. Stupidly injecting everything can eventually be a feature? e.g.

XDG_CURRENT_DESKTOP=GNOME
command=gnome-control-center

An option can to be to uppercase and/or prefix every key, but the downside is that the user will have to know about this transformation and play with it, e.g. echo "{ ... \"foo\": \"$_FOO\" ... }".
Another option would be to filter out which property to inject, e.g. marking i3 and i3blocks specific keys, but I'd rather not hardcode any properties if possible and follow a simpler convention.

A simple convention can be to inject only uppercase properties. Thus foo wouldn't be injected but FOO will. This seems more intuitive from the config and process point of view. We'd keep the existing BLOCK_* variables for a bit for retro-compatibility for sure.

Does this look OK? Is there another alternative?

vivien referenced this issue Jan 2, 2018
Now that we have dynamic properties, it'd be nice to set all of them
into the environment of a forked child.

To keep it simple for the moment, just upcase and prefix the key with
"BLOCK_". We might deprecate that for a consistent convention later.

This provides a simple way to pass arguments to script like this:

    [foo]
    bar=baz
    command=echo $BLOCK_BAR
    interval=2
@vivien
Copy link
Owner Author

vivien commented Jan 2, 2018

Note: how to inject click info? It is currently set as BLOCK_X, BLOCK_Y, and BLOCK_BUTTON.

@jolange
Copy link

jolange commented Jan 2, 2018

A simple convention can be to inject only uppercase properties. Thus foo wouldn't be injected but FOO will.

I prefer this over automatically upcasing every key. That would certainly lead to confusions.

how to inject click info?

Replacing "BLOCK" with "CLICK" would be more intuitive to me, but I'm not sure if it's worth introducing a compatibility breaking change.

@vivien
Copy link
Owner Author

vivien commented Jan 2, 2018

Hi @jolange. I have no problem with introducing a new convention, because I'll keep the BLOCK_ variables for a few releases anyway (which will be deprecated if we choose a new convention.)

It is worth noting that a few properties need to be injected as well, including i3-specific name and instance and click properties x, y, and button sent by i3. I'm not sure if other i3blocks or i3 specific properties need to be injected (e.g. full_text, signal, etc.), but injecting them all wouldn't hurt.

The current implementation in the next branch (commit 09d8db1) is using uppercase and a BLOCK_ prefix. A question is, should we (and how) make a distinction between i3/i3blocks properties and user-provided keys?

What can be currently confusing for a user is to provide this:

[blocklet]
foo=bar
command=foo.sh

and access it from foo.sh with $BLOCK_FOO and eventually update back foo (instead of BLOCK_FOO.)

If we stupidly inject everything, then a script's environment will have variables such as $x, $button, $name, $instance, $FOO, etc. which would be the most intuitive for a user (except the fact that i3blocks won't update some properties such as command or signal, etc. This doesn't define any convention but it doesn't prevent a user from defining one.

@jolange
Copy link

jolange commented Jan 3, 2018

I think setting environment variables for the i3blocks specific stuff (command, instance, ...) is fine, because these are obvious from the configuration and there is no need to treat them differently from user properties.

Concerning things that are set by i3 the situation is a bit different, because the user might not be aware of those. Are you saying that you have to set e.g. x, y, button as environment variables (with the values sent by i3)? That might cause problems if a user accidently overwrites those with an own property.

The foo.sh example would indeed not be very intuitive.

@vivien
Copy link
Owner Author

vivien commented Jan 3, 2018

Are you saying that you have to set e.g. x, y, button as environment variables (with the values sent by i3)?

This is how it is done today, i3 sends x, y, and button (along with the concerned block's name and instance properties), and i3blocks injects respectively BLOCK_X, BLOCK_Y, and BLOCK_BUTTON in the process environment.

Another option could be to inject only user properties using the form BLOCK_UPPERCASENAME, e.g.:

[block1]
BLOCK_VAR1=foo
BLOCK_VAR2=bar
baz=xxx
command=myscript.sh
interval=2

BLOCK_VAR1 and BLOCK_VAR2 would be passed to myscript.sh but not baz.

@jolange
Copy link

jolange commented Jan 4, 2018

OK, I misunderstood that. Then currently my personal preference would be CLICK_X, CLICK_Y, CLICK_BUTTON. For the rest, I don't see any disadvantage in injecting all keys as they are. The only problem could be that people accidently overwrite existing environment variables, but I don't see this causing larger issues.

@kb100
Copy link
Contributor

kb100 commented Jan 5, 2018

I think passing all variables is the way to go

vivien added a commit that referenced this issue Aug 21, 2018
Now that we have dynamic properties, it'd be nice to set all of them
into the environment of a forked process.

This provides a simple way to pass arguments to script like this:

    [foo]
    bar=baz
    command=echo $bar
    interval=2

Refs #279
vivien added a commit that referenced this issue Sep 4, 2018
Now that we have dynamic properties, it'd be nice to set all of them
into the environment of a forked process.

This provides a simple way to pass arguments to script like this:

    [foo]
    bar=baz
    command=echo $bar
    interval=2

Refs #279
@vivien
Copy link
Owner Author

vivien commented Sep 7, 2018

All properties are passed as is as environment variables. This means foo=bar from the configuration file will inject e.g. a $foo variable from a shell script. This is the responsibility of the user to inject properly name variable. Anything is accepted, but using an UPPERCASE convention of _underscore prefixed convention as stated by i3bar would be preferable.

@vivien
Copy link
Owner Author

vivien commented Jan 1, 2019

As a side note, IEEE states that "The name space of environment variable names containing lowercase letters is reserved for applications. Applications can define any environment variables with names from this name space without modifying the behavior of the standard utilities." so I'm happy to see that passing all variables as is, including lowercase i3bar/i3blocks properties, was the way to go. I've added this quote in the FAQ.

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

3 participants