Add window arguments

[WilfSilver]

I've just created this for my config to work so thought I would create a PR to see if this is wanted in the master branch. If it is wanted, I shall flesh out the features a bit more + add docs and update tests

Description

The main goal of these changes were to make to use the same configuration for multiple bars on different screens

There are two features that I implemented in this as they seemed somewhat similar/linked

• The addition of widget like arguments for defwindow, which can be specified when opening a window by function like parameters e.g. window(param1,param2). (this was the first format I was able to come up with, but happy for suggestions, I wanted to allow it so it can be written without spaces.). The config for that would look like: (defwindow [arg1, arg2] ...)
  • I have made sure, however, that previous configs which don't include [] do still work
  • I have also updated it so that monitor option can use these local variables, I think we could also update it so that it can update other options, however I was only looking for setting the screen through this option
  • These options are then used as the unique identifier for the window of a sorts so that you can open up multiple windows as long as the parameters for each one is different as it uses the full window(param1,param2) when saving open windows as well as the scope
• Per window variable option: This allows variables to have :per_window true option set (the default is false) which will mean that the variable will be stored separately for each window. To do this I have refactored the super_scope out into an argument for the build functions so that it has to be specified, which then allows to create a specific "global" scope for each window, which then all the widgets link to instead of directly to the root scope.
  • To then solve the issue of updating these variables as they are no longer stored in the root scope, I added a --window option to the update command which allows you to specify the window this is for e.g. --window primary, or to also include an example of a window with parameters --window "secondary(param1,param2)"

Usage

Kind of done this above but will show some better examples:

For the window parameters, I use this for a secondary bar, so that whatever number of monitors I have I can have a simple script get the number of monitors connected and generate a secondary bar for each monitor which is not the primary one

(defwindow secondary [monitor]
          :geometry (geometry
                       :x      "0%"
                       :y      "6px"
                       :width  "100%"
                       :height "30px"
                       :anchor "top center")
          :monitor    monitor
          :stacking   "bg"
          :windowtype "dock"
          :reserve    (struts :distance "50px" :side "top")
    (secondary_bar :window_num monitor :eww_update "${eww} update --window 'secondary(${monitor})'"))

Then my current launch script for each bars uses a function to generate the arguments and spits them in a command:

run_eww() {
    secondaries=($(get_secondary_bars))
    ${EWW} open-many \
        primary ${secondaries[@]}
}

get_secondary_bars() {
    raw_info=$(xrandr --listactivemonitors | grep "Monitors: ")
    num_monitors=${raw_info/Monitors: /}
    names=()
    for ((i = 1; i < $num_monitors; i++))
    do
        names[$i]="secondary($i)"
    done
    echo "${names[@]}"
}

So then for the local variable, when having an expanding option for a configuration, such as volume control, it allows the bars to work separately without having to have separate instances running in the background. So, currently my config for the volume widget looks like (99% stolen from saimoom's config):

(defvar vol_reveal :per_window true false)

(defwidget volume [eww_update]
  (eventbox :onhover "${eww_update} vol_reveal=true"
              :onhoverlost "${eww_update} vol_reveal=false"
  (box :class "module-2" :space-evenly "false" :orientation "h" :spacing "3"
    (button   :onclick "scripts/pop audio"   :class "volume_icon" "")
    (revealer :transition "slideleft"
              :reveal vol_reveal
              :duration "350ms"
        (scale    :class "volbar"
                :value volume_percent
                :orientation "h"
                :tooltip "${volume_percent}%"
                :max 100
                :min 0
                :onchange "amixer -D pulse sset Master {}%" )))))

As you can see it uses the variable eww_update to update the volume, which is set above in the secondary config and passed down through the widgets. This then allows to get results such as shown in the showcase where one volume widget is showing the full slider where the other is not.

Showcase

Apologies for the bad screenshot, my monitors are not aligned but they get the point across

As you can see the bar on the left is showing the volume slider whereas the bar on the right is not, when both are running through the same instance of eww and also using the same widget shown above in the configuration

Additional Notes

I guess I shall include my idea of "fleshing" the features out here.

• I'm pretty sure that having multiple window arguments breaks everything somehow... I only briefly tried it once when experimenting with something else

Would love some feedback on these:

• I think my next step would be to allow more options to be set by local variables, e.g. stacking or windowtype , or anything set in the geometry
• Error messages, mainly relating to the above as I think currently it won't give the right error, I didn't particularly check, because you can't use global variables in the defwindow, instead being stuck with only the local variables defined through the arguments
• Cleaning up the use of AttrSpec as defwindow does use it but it currently doesn't use the optional variable. This might be better off moved back so only widget_use uses it and instead window_definition uses just a simple Vector of AttrName
• Current extraction of window parameters is a bit basic and quite strict of what it needs to that should probably be loosened up.
• Also I shall write this down here, I would love to take suggestions for the current syntax/calling of a window with arguments as it doesn't particularly conform to the current way of specifying arguments, however I did want this to work smoothly with open-many command
• I think I have broken one of the tests, when I was skimming through some code, due to the change of making monitor__num a SimplExpr instead of an i32, I've probably broken a lot more, that is just the one I noticed.
• Also I should probably add some tests

Question:

• I have added args_span to the WindowDefintion to be consistent with the WidgetDefintion , however just looking at it, it seems that this is only referenced in the validate.rs as debugging stuff, which I do not use so would this actually be necessary/should I remove it?

Checklist

I've left most of these blank for now as I wanted to check these features are wanted before updating docs and stuff as I was not able to find anyone mentioning/asking for these features. (Also why its a draft PR)

• Experiment with/Fix the bug related to having multiple arguments
• Flesh out/clean up the code
• Do more testing
• Update/add tests as I think I have broken some
• All widgets I've added are correctly documented.
• I added my changes to CHANGELOG.md, if appropriate.
• The documentation in the docs/content/main directory has been adjusted to reflect my changes.
• I used cargo fmt to automatically format all code before committing (I must admit this is sad that it gets rid of aligned aligned code)

EDIT: the per window variables have been moved off this MR as a similar but better implementation is currently being worked on, to see my version of it see https://github.com/WilfSilver/eww/tree/per_window_variables

[druskus20]

oh wow

[elkowar]

Damn, this is huge! Love it, and thanks a lot already for all the effort you put into this! I was planning to implement something like this before, already, very cool!

Obviously this is going to take some time for me to think about, both in terms of design decisions and implementation detail, but here's a few initial thoughts:

• I don't really like the approach you're currently going for with how to call and identify the windows. I'd argue that a cleaner approach would be to require the user to specify some explicit, unique identifier for a window if they want to open multiple windows of the same kind, and then to just use that as the ID (rather than going by the values of all the arguments)
• I really don't like the idea of having this sort of "function-call like" window calling API for the command line. What I was thinking when I initially considered something like this was to go with only having named arguments, and then going with something along the lines of

  eww open-many \
    foo --monitor 1 --arg1 value --arg2 value2 \
    bar --monitor 2 --arg1 value2 --arg2 foobar
  

  or something along those lines.
• I am still very much planning to implement [FEATURE] widget-local state #324 in the not-tooooooo distant future (I hope,....). The implementation of window-local state / variables may very much change based on some more core concept of local state being introduced to eww -- not sure (haven't looked through your code enough yet). Depending on that, It may be worth it to wait until local state is implemented, as that directly impacts window-local scopes.
• I do think we should then allow users to reference the window-variables in all the window definition attributes

[WilfSilver]

Thanks for the feedback!

So with the first point about having a separate argument that is enforced with a window id, do you think it should look something like this (with the structure in the second point):

eww open-many \
  foo --id first_foo --monitor 1 ... \
  foo --id second_foo --monitor 2 ...

I do agree that would be cleaner option, and would be a lot nicer when trying to update variables for a given window id. The only issue I can see with this is that I'm currently using the id of the window to regenerate the arguments when a config is loaded. Which links to an interesting issue I just randomly stumbled across, which is that --screen option is not saved so when you go to reload the script, the bar swaps to the primary display. So I could try and fix that bug while also allowing an id to be given instead. My first thought is to have those options saved inside the open_windows hash table somewhere and then when reloading the config we can look at those variables and send it to the open_window function. What do you think?

I also agree with the second point with the args as then it would allow you to use the names of the variables instead of its positions, and would probably fit with the current system of attributes a lot better.

With #324, that would be awesome, and would solve the problem I was trying to fix with the per window variables. I'm trying to figure out if that would make the per window option redundant, they both can serve different things but I am failing to come up with a good use case for the per window global variables. If you have any ideas, would love to hear them. But, I think it might just be best to remove it as the per window variables are a bit of an awkward middle ground. But I agree waiting for #324 to be implemented first would be good as I'm guessing that would change quite a bit of the scope structure.

However I do have one question about it, would it allow a widget further down the stack to set the variable of a widget further up the stack? I'm guessing this would have to be done through passing the variable down the stack, as otherwise the syntax validator would have a really hard time.

In the mean time I can fix the tests and update the commands for creating a window.

[elkowar]

yea, exactly, that's how the IDs would have to look

I think that if we had #324 we'd not need global per-window variables, those would just need to be passed down from the window to the widgets.
I'm not sure if I'd allow mutating local variables from within child widgets, I hadn't really thought about that yet (As in, I haven't really thought through mutation in general yet, to be honest)

[WilfSilver]

Just a quick update I've been researching how to do the arguments with the structopt package. I haven't used this package before, especially in this context, so I may have got things wrong, please say if I have!

From what I have read I don't think it will be possible to have something like.

eww open-many \
    foo --monitor 0 --arg1 value1 --arg2 value2 \
    bar --monitor 1 --arg1 value1 --arg2 value2

After reading this post

To summarise (from my understanding), you can't have something like windows: Vec<OpenWindowArgs> where OpenWindowArgs def looks like:

#[derive(StructOpt, Debug, Serialize, Deserialize, PartialEq)]
struct OpenWindowArgs {
    window_name: String,

    #[structopt(long)]
    id: Option<String>,

    /// Monitor-index the window should open on
    #[structopt(long)]
    screen: Option<i32>,

    /// The position of the window, where it should open.
    #[structopt(short, long)]
    pos: Option<Coords>,

    /// The size of the window to open
    #[structopt(short, long)]
    size: Option<Coords>,

    /// Sidepoint of the window, formatted like "top right"
    #[structopt(short, long)]
    anchor: Option<AnchorPoint>,
}

Secondly, I think it would be very hard to get the command below working

eww open foo --monitor 0 --arg1 value1 --arg2 value2

There may be a way if you override start overriding the clap and from_clap functions in an implementation, however we would have to have access to the config before it has been extracted from the arguments (if you wanted to have the options always match the window you have put in)

So I have two simpler alternatives, both of which are definitely nowhere near as nice though.

Alternative 1

Using similar logic to the current update subcommand, where we have a --args option for open the open subcommand, so the example from above would look like:

eww open foo --args monitor=0 arg1=value1 arg2=value2

The only issue which comes with this is that open-many would be incompatible, so you wouldn't be able to open windows with arguments with that command.

Alternative 2

One that is probably even more disgusting, but would work with open-many, is a similar thing to defining arguments for widgets:

eww open-many \
    "foo :monitor 0 :arg1 value1 :arg2 value2" \
    "bar :monitor 1 :arg1 value1 :arg2 value2"

Technically you could swap : with -- but I think that would make it a bit too confusing...

An issue which appears with this one is that would you want to move the options screen, pos, size, anchor into arguments you define via this method so you could define the screen for each option in the open-many command.

Anyway, if I'm being stupid and missing something in stuctopts, please tell me because if we could do the custom argument idea, I definitely would prefer it.

Other ideas are also welcome :)

[elkowar]

Yea, I've been thinking about that structopt limitation too, I fear -- I think there's an issue about that open somewhere, but nothing happened in a loooong while.
I think alternative 1 would be doable, although not having open-many support would be very sad. I'm sure there should be some way to make it at least acceptable,...

worst-case yea, the strat would be to do a partly custom command line argument parsing thing,.. but PAIN

[WilfSilver]

Just quick proposal as I just came up with a horrid(ish) way of making the --args method work with open-many, this is most definitely a work around.

So basically for open-many you can have a parameter named --args and that takes in a list of arguments to pass to the windows, but the catch here is that the id has to be specified before each variable. The only issue here being is that we can't specify an id for each window (or can we?)

E.g.

eww open-many foo bar --args \ 
    foo:monitor=0 foo:arg1=value1 foo:arg2=value2 \
    bar:monitor=1 bar:arg1=value1 bar:arg2=value2

And if we did want to define an id for each window, following the common theme, we can do something like:

eww open-many foo:foo1 foo:foo2 --args \ 
    foo1:monitor=0 foo1:arg1=value1 foo1:arg2=value2 \
    foo2:monitor=1 foo2:arg1=value1 foo2:arg2=value2

I guess the only real downside to this (other than how hacky it is) is that its a slight pain to automate as you have to have each variable have the id put before

[WilfSilver]

Just quick proposal as I just came up with a horrid(ish) way of making the --args method work with open-many, this is most definitely a work around.

I have now implemented a version of this to experiment with, I have made it so it is backwards compatible and seems to work pretty well so far.

I have also added documentation and added to the changelog for the arguments so I think we are just wating to see what happens with #440.

I have not altered the per window variables, so I think I will move that change out of this branch so it doesn't get confused with this change - but will probably keep in in another branch on my fork.

[WilfSilver]

As a quick update to this: A (somewhat) recent change made me reconsider the implementation of this.

So I have renamed and reorganised some things:

• WindowInitiator is now named WindowArguments and is just there to store the arguments required to run open_window. This has also been moved into its own file
• I have also added a new WindowInitiator which stores the information which we need once we have gotten the WindowDefinition, which makes it easy to pass around when creating a window (the definition was used to pass around but we can't really do that when the variables are just SimplExprs.

This new change just makes it easier to have configurations in WindowDef be SimplExpr and so they can be customised and rely on the window parameters.

Currently doing some more testing but hopefully I can now easily transfer some more options to use SimplExpr in the window definition.

[WilfSilver]

I have now implemented all options to use SimplExpr which is based off of the window parameters. Example of the config which is now possible:

(defwindow primary [id gui_size]
          :geometry (geometry
                       :x      "0%"
                       :y      { gui_size == "small_4k" ? "4px" : "6px" }
                       :width  "100%"
                       :height { gui_size == "small_4k" ? "30px" : "40px" }
                       :anchor "top center")
          :monitor 0
          :stacking   "bg"
          :windowtype "dock"
          :focusable false
          :reserve    (struts :distance { gui_size == "small_4k" ? "84px" : "52px" } :side "top")
    (primary_bar :eww_update "${eww} update --window ${id}" :size gui_size :window_num 0))

[WilfSilver]

I have also added a small bug fix, which I couldn't find reported yet, where the struts don't take into account the scale factor when being created. So on 2 monitors with gtk interface scaling set to 2, the struts are in incorrect locations (meaning on the second monitor the strut is created on the first monitor but displays on the second).

Feel free to cherry-pick or copy into another MR to merge quicker :)

[elkowar]

Hey! So, after months of not getting to this, I finally managed to do an in-depth review, and I figured the easiest thing to do is to just do some cleanup as I go through the code, committing my changes as I go. Please look over those last few commits and check if theres anything that you strongly disagree with, or where I might be missing something.

There's a few small open questions still:

• we need to update the eww windows command to deal with the fact that there can now be multiple instances of a window, and make the logic handle the fact that windows are stored by name, not id. Not sure how possible that is without a breaking change, but I'd be fine with having a breaking change here if necessary
• I'm not a big fan of the name of window_argumentss, eventhough it's quite the funny solution to that naming problem -- idk
• I think the documentation should definitely clarify that the arguments cannot be changed after the window has been opened -- the fact that there is no eww update support here might be confusing otherwise.

[WilfSilver]

Thanks for cleaning up the code a bit, looks a lot better.

• we need to update the eww windows command to deal with the fact that there can now be multiple instances of a window, and make the logic handle the fact that windows are stored by name, not id. Not sure how possible that is without a breaking change, but I'd be fine with having a breaking change here if necessary

Just going over the implementation for this and it seems that there may not be a nice way to implement this...

Currently I'm thinking we want to show the instances which are using the window name so have something in the form:

*window_name (id1, id2, id3)
closed_window

However this may be a bit of a pain to have a script read the information

Then there is the implementation...

Currently it seems to produce the intended result there are two methods:

• If we are assuming that this is not going to be run too often and so speed doesn't matter, we could just implement a function to go through all the open windows and gather the ids which have this window open
• The other method is to have a cache which would mean that it needs to be maintained when editing the open windows (which probably could just be done through forcing people to use functions instead)

I'm kinda more leaning towards the first method as its easier but can do the second (or another) if you prefer.

• I think the documentation should definitely clarify that the arguments cannot be changed after the window has been opened -- the fact that there is no eww update support here might be confusing otherwise.

Cool will update the documentation now

[elkowar]

RE eww windows:
I was talking to @buffet about this, and we came up with the following output for easy usage in scripts:

$ eww windows
bar-1: bar
bar-2: bar

which would indicate that there's two instances of bar, one with id bar-1 and one with id bar-2
This would make the output noticably easier to parse in scripts, as you can just go over it liine-by-line.
However, this would now also not include the non-open windows -- so maybe split the command into
eww windows --active and eww windows, where the latter just lists the window names, without indicating if a window is opened or not.

This would be a breaking change, but would maximise the usability of this API in the future, I'd think

[elkowar]

Pushed an implementation of that API

[elkowar]

I HIT MERGE IT FINALLY HAPPENED OMG OMG OMG OMG OMG

Congrats man! Terribly sorry this took so horribly long, Idk, can't really explain it except me being super distracted and busy all the time,....
But I'm very hyped this is done now. This is probably the biggest thing to happen in eww in a year! Thanks a lot for your work!