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

river-options: create protocol #202

Merged
merged 2 commits into from
Jan 16, 2021
Merged

river-options: create protocol #202

merged 2 commits into from
Jan 16, 2021

Conversation

ifreund
Copy link
Member

@ifreund ifreund commented Jan 14, 2021

The idea with this protocol is to expose a key value store allowing clients to set and retrieve arbitrary configuration options as well as watch for changes in options. This would provide a simple mechanism to configure layout generators in an extensible way.

It is also planned to move any of river's simpler configuration options such as border width or background color over to this
protocol.

TODO:

  • figure out how to implement output/seat specific options and if we really want them.

See also this discussion on IRC: https://freenode.irclog.whitequark.org/river/2021-01-14#1610623485-1610638349;

cc @Leon-Plickat

@Leon-Plickat
Copy link
Member

figure out how to implement output/seat specific options and if we really want them.

We definitely want them. Things such as main amount should be per output,.


What we also need to think about is how to trigger layouts if layout relevant values change. Currently we just issue a layout demand if one of the values change. However with the value store river looses the concept of things such as main views.

Maybe we need a way for clients to trigger a layout and then let them just subscribe to changes of these values.

@Leon-Plickat
Copy link
Member

IRC dump:

ifreund: leon-p: should we perhaps do namespacing under an arbitrary string
         rather than wl_output or wl_seat object? It would be a lot more
         flexible
ifreund: could then use the seat name or xdg_output name for example, or
         anything else we might not have thought of yet
leon-p: hmm... not sure tbh. Spontaniously I'd say it needs to be output
        and seat bound. But it could still be a global list, where each entry
        simply has a wl_object and a wl_seat pointer, which could be NULL.
leon-p: I'd generally prefer to have as little string operations as possible.
        If we use the xdg-output name as a string, clients would need to
        allocate a buffer with it
leon-p: so my idea is to have a `struct { key: []const u8, seat: ?*wl.Seat,
        output: ?*wl.Output, value: union { ... }, };`
leon-p: that would be the most flexible IMO, as it allows clients to have a
        key-value-pair be bound to either/or/and/none of the output and seat.
leon-p: As an example, the main factor would have an output pointer, but
        seat == null.
leon-p: While the keymap would have a seat pointer, with output == null

@Leon-Plickat
Copy link
Member

Leon-Plickat commented Jan 15, 2021

Counter proposal, which I think would be nicer to work with from a clients PoV. Especially having multiple different events for the different value types only works nice with zig-waylands kind of even handlers, while that would be pretty annoying to use in C. And there really needs to be a way for a client 1) to know of what type the option it retrieved is to allow clients to safely handle the case where an option has an unexpected type and 2) to know when an option has not been populated, which is important to clients which may only want to use river-config as an override for their own internal defaults and/or configuration mechanism (consider that 0 and NULL, the default values of options when unpopulated, are ambiguous as they might be otherwise correct values for this option).

<?xml version="1.0" encoding="UTF-8"?>
<protocol name="river_config_unstable_v1">
  <copyright>
    Copyright 2021 The River Developers

    Permission to use, copy, modify, and/or distribute this software for any
    purpose with or without fee is hereby granted, provided that the above
    copyright notice and this permission notice appear in all copies.

    THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
    WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
    MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
    ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
    WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
    ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
    OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
  </copyright>

  <description summary="store and retrieve key-value-pairs">
    This protocol specifies a way for clients to register arbitrary information
    into a key-value-pair database as well as retrieve mentioned information
    and subscribe to its changes.

    The database has a per-session lifetime and a global scope.
  </description>

  <enum name="value_type">
    <entry name="int" value="0"/>
    <entry name="uint" value="1"/>
    <entry name="fixed" value="2"/>
    <entry name="string" value="3"/>
  </enum>

  <interface name="zriver_option_manager_v1" version="1">
    <description summary="store and retrieve options">
    </description>

    <request name="destroy" type="destructor">
      <description summary="destroy the zriver_option_manager_v1 object">
        This request indicates that the client will not use the
        zriver_option_manager_v1 object any more. Objects that have been created
        through this instance are not affected.
      </description>
    </request>

    <request name="declare_option">
      <description summary="declare a new option">
        Declare a new option with a given type. This may silently fail if
        the option already exists. The initial value of the option depends
        on the type: for numeric types the initial value is 0, for strings
        the initial value is NULL.

        Multiple options may share the same key, if their seat and output
        combination is different.
      </description>
      <arg name="key" type="string"/>
      <arg name="output" type="object" interface="wl_output" allow-null="true"/>
      <arg name="seat" type="object" interface="wl_seat" allow-null="true"/>
      <arg name="type" type="uint" enum="value_type"/>
    </request>

    <enum name="error">
      <entry name="option_does_not_exists" value="0"
        summary="tried to retrieve an option which does not exist in the database"/>
    </enum>

    <request name="get_option">
      <description summary="retrieve an option">
          Get a river-option-v1 matching the given key output and seat
          combination.

          Trying to retrieve an option which does not exists is a protocol
          error. Therefore client authors are advised to always explicitly
          declare any options they wish to use unless they know for certain
          that such an option exists by the time it is retrieved by the client.
      </description>
      <arg name="key" type="string"/>
      <arg name="output" type="object" interface="wl_output" allow-null="true"/>
      <arg name="seat" type="object" interface="wl_seat" allow-null="true"/>
      <arg name="option" type="new_id" interface="zriver_option_v1"/>
    </request>
  </interface>

  <interface name="zriver_option_v1" version="1">
    <description summary="">
      On binding this object, one of the events will immediately be sent by
      the server. The type of value of an optioin is immutable Making a set
      value request of a different type is a protocol error.
    </description>

    <enum name="error">
      <entry name="type_mismatch" value="0"
        summary="the client made a set value request of the wrong type."/>
    </enum>

    <request name="destroy" type="destructor">
      <description summary="destroy the zriver_option_v1 object">
        This request indicates that the client will not use the
        zriver_option_v1 object any more.
      </description>
    </request>

    <event name="value">
      <description summary="value changed">
          This event is send if the value of an option changed and on bind.

          Only the argument related to the options type is populated by
          useable data. All other type specific arguments will be 0 or NULL.

          If this option has never been populated, meaning it has been declared
          but no client has set its value, the unpopulated argument is set to
          true.
      </description>
      <arg name="type" type="uint" enum="value_type"/>
      <arg name="int_value" type="int"/>
      <arg name="uint_value" type="uint"/>
      <arg name="fixed_value" type="fixed"/>
      <arg name="string_value" type="string" allow-null="true"/>
      <arg name="unpopulated" type="bool"/>
    </event>

    <request name="set">
      <description summary="set the value of an option">
          This request can be used to set the value of an option.

          Only the argument related to the options type will be considered.
          All other arguments will be ignored.
      </description>
      <arg name="int_value" type="int"/>
      <arg name="uint_value" type="uint"/>
      <arg name="fixed_value" type="fixed"/>
      <arg name="string_value" type="string" allow-null="true"/>
    </request>
  </interface>
</protocol>

@Leon-Plickat
Copy link
Member

I think it would make sense to first finish #112, removing the main view related values from the protocol and river and rivertile and contrib/layout.c (the latter two keeping placeholders).

Then when river-config is implemented, the only layout related changes would be implementing river-control in rivertile and contrib/layout.c and to set some default values for the main views in the default init.

@ifreund
Copy link
Member Author

ifreund commented Jan 15, 2021

Counter proposal, which I think would be nicer to work with from a clients PoV. Especially having multiple different events for the different value types only works nice with zig-waylands kind of even handlers, while that would be pretty annoying to use in C.

One could use the same function to handle all the undesired cases in C, similar to the noop function used here. The big disadvantage to having only a single event/request type as you propose is that it makes the protocol considerably less efficient as 12 useless bytes are sent with every event/request.

And there really needs to be a way for a client 1) to know of what type the option it retrieved is to allow clients to safely handle the case where an option has an unexpected type

The client is guaranteed to receive a *_value event on binding the option which indicates the type. Compliant clients would never make a request before receiving this first event as the type of the option would not yet be known. If they receive a *_value event of a undesired type, they can safely destroy the zriver_option_v1 and perhaps log an error.

2) to know when an option has not been populated, which is important to clients which may only want to use river-config as an override for their own internal defaults and/or configuration mechanism (consider that 0 and NULL, the default values of options when unpopulated, are ambiguous as they might be otherwise correct values for this option).

hmm, this could be solved in a couple ways:

  • introduce an unpopulated event sent on binding the zriver_option_v1 containing nothing but a value_type enum and get rid of the default values specified in declare_option.
  • remove the type argument of the declare_option request and have options initially have no type until a value is set. This is racy though if 2 clients attempt to set the initial value of the option to different types at the same time. We would need to get rid of the protocol error for that case and state that attempting to set with the wrong type is simply ignored. If we do this we could also just get rid of the declare_option request entirely. I think this is the better idea.

I think it would make sense to first finish #112, removing the main view related values from the protocol and river and rivertile and contrib/layout.c (the latter two keeping placeholders).

I'm pretty confident that this idea of a key-value store is a good one so yeah we should definitely remove the main area related values from the protocol.

@ifreund ifreund force-pushed the river-config branch 3 times, most recently from c6770db to b9efe66 Compare January 15, 2021 15:28
@ifreund ifreund changed the title river-config: create protocol river-options: create protocol Jan 15, 2021
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
</copyright>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a global <description ...> would be nice.

We also need to explain the life-time of the list and I think that would be a good place for it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does "Options may never be unset once set." not cover that?

@ifreund ifreund force-pushed the river-config branch 3 times, most recently from ff20575 to 2953f7c Compare January 16, 2021 19:38
@ifreund ifreund marked this pull request as ready for review January 16, 2021 21:15
@ifreund
Copy link
Member Author

ifreund commented Jan 16, 2021

Merging this as the sever side implementation is complete and has been tested with a (crappy) example client. Implementation in riverctl and migration of current options to this system can happen in a future PR.

@ifreund ifreund merged commit 8cbccbf into master Jan 16, 2021
@ifreund ifreund deleted the river-config branch January 16, 2021 22:51
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

Successfully merging this pull request may close these issues.

2 participants