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

Refactor the config parser into a library #2858

Closed
acrisci opened this Issue Aug 13, 2017 · 17 comments

Comments

Projects
None yet
8 participants
@acrisci
Member

acrisci commented Aug 13, 2017

The current config parser is kind of a mess right now and could use some work to ease maintenance in the future.

To support a richer desktop environment experience for i3, I propose that we make this library available to users for their own projects. This will promote maintenance of the library by bringing in more parties who are interested in the proper functioning of the library.

Here are some possible external users of this library:

  • Dock clients that interact with i3 such as xfce4-i3-workspaces-plugin.
  • Config editors that want to display a representation of the current colorscheme (i3-style).
  • Other window managers like sway that use config syntax related to i3 (that could be supported through built-in extensions).

There are probably other great use cases people will think of once this interface is exposed to users. If you build it, they will come.

If the library is written with GObject (included with GLib), we can use introspection to give the library to users as a python library to support user scripting.

cc @SirCmpwn

@i3bot

This comment has been minimized.

Show comment
Hide comment
@i3bot

i3bot Aug 13, 2017

I don’t see a link to logs.i3wm.org. Did you follow http://i3wm.org/docs/debugging.html? (In case you actually provided a link to a logfile, please ignore me.)

i3bot commented Aug 13, 2017

I don’t see a link to logs.i3wm.org. Did you follow http://i3wm.org/docs/debugging.html? (In case you actually provided a link to a logfile, please ignore me.)

@i3bot i3bot added the missing-log label Aug 13, 2017

@i3bot

This comment has been minimized.

Show comment
Hide comment
@i3bot

i3bot Aug 13, 2017

I don’t see a version number. Could you please copy & paste the output of i3 --version into this issue?

i3bot commented Aug 13, 2017

I don’t see a version number. Could you please copy & paste the output of i3 --version into this issue?

@SirCmpwn

This comment has been minimized.

Show comment
Hide comment
@SirCmpwn

SirCmpwn Aug 13, 2017

Other window managers like sway that use config syntax related to i3

We already have a pretty solid i3 config parser, I doubt the change overhead would be worth it for us.

If the library is written with GObject (included with GLib)

Then we would definitely not use it 😉

SirCmpwn commented Aug 13, 2017

Other window managers like sway that use config syntax related to i3

We already have a pretty solid i3 config parser, I doubt the change overhead would be worth it for us.

If the library is written with GObject (included with GLib)

Then we would definitely not use it 😉

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Aug 13, 2017

Member

I support this idea. I even wonder whether we should move away from C for this. Rust is something often recommended these days, especially for things like parsers. I'm not sure @stapelberg would be open for this, however.

Member

Airblader commented Aug 13, 2017

I support this idea. I even wonder whether we should move away from C for this. Rust is something often recommended these days, especially for things like parsers. I'm not sure @stapelberg would be open for this, however.

@stapelberg

This comment has been minimized.

Show comment
Hide comment
@stapelberg

stapelberg Aug 13, 2017

Member

I’m not opposed to Rust, provided you feel comfortable maintaining Rust code (my Rust knowledge is quite limited).

If the parser library was written in Rust, would it be usable from all languages which have a C FFI?

One thing which wasn’t mentioned so far is whether the library would live in a separate git repository. My intuition would be that it should live in the same repository as i3, much like the IPC type headers, to keep version skew to a minimum.

We should carefully review the interface we want to provide (regardless of implementation language), to make sure we don’t commit to maintaining a flawed interface for a long time.

Member

stapelberg commented Aug 13, 2017

I’m not opposed to Rust, provided you feel comfortable maintaining Rust code (my Rust knowledge is quite limited).

If the parser library was written in Rust, would it be usable from all languages which have a C FFI?

One thing which wasn’t mentioned so far is whether the library would live in a separate git repository. My intuition would be that it should live in the same repository as i3, much like the IPC type headers, to keep version skew to a minimum.

We should carefully review the interface we want to provide (regardless of implementation language), to make sure we don’t commit to maintaining a flawed interface for a long time.

@Airblader

This comment has been minimized.

Show comment
Hide comment
@Airblader

Airblader Aug 13, 2017

Member

I also don't really have knowledge of Rust as of yet. At the moment it's a mere proposal, I doubt the split if the parser will happen over night anyway. You're absolutely right that we need to review these decisions carefully.

I do believe that parsers are supposed to be relatively simple to write in Rust. I could imagine Rust code might be easier to maintain here than C code even if you have more knowledge with the latter. Parsers in C just tend to become messy.

Member

Airblader commented Aug 13, 2017

I also don't really have knowledge of Rust as of yet. At the moment it's a mere proposal, I doubt the split if the parser will happen over night anyway. You're absolutely right that we need to review these decisions carefully.

I do believe that parsers are supposed to be relatively simple to write in Rust. I could imagine Rust code might be easier to maintain here than C code even if you have more knowledge with the latter. Parsers in C just tend to become messy.

@acrisci

This comment has been minimized.

Show comment
Hide comment
@acrisci

acrisci Aug 13, 2017

Member

Rust could be a problem for adoption by desktop environment applications and other users if it means they must add a dependency on rust. The extension community likes "pure" interfaces as much as possible with few dependencies. The major languages that need to be supported are C and python. So if GObject isn't desirable, a very thin C to python library wouldn't be hard to write.

Member

acrisci commented Aug 13, 2017

Rust could be a problem for adoption by desktop environment applications and other users if it means they must add a dependency on rust. The extension community likes "pure" interfaces as much as possible with few dependencies. The major languages that need to be supported are C and python. So if GObject isn't desirable, a very thin C to python library wouldn't be hard to write.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Aug 20, 2017

Maybe you want to consider going even a step further: Instead providing yet another config library, you/we could put your parsing code as plugin in Elektra.

We already have a pretty solid i3 config parser, I doubt the change overhead would be worth it for us.

Every other application using Elektra could also use your format, so we love to get new good parsers. With the plugin in place, i3 could keep the same syntax by default. Nevertheless, with Elektra i3 users could also choose their own favorite configuration file format (such as INI, json, XML,..).

If the library is written with GObject (included with GLib)
Then we would definitely not use it 😉

Libelektra does not use GObject in its core (only C, no dep), but it has bindings for GObject and gobject-introspection (next to other bindings such as python, java, lua, ruby, C++).

Elektra also already provides quite some tooling such as command-line tools, graphical editors, and soon web frontends. You should not underestimate the effort to provide such an environment. There is much more involved to it than having a C-library ;) In particular providing proper configuration validation is quite a heavy job: but essential for a good user experience.

Rust

We currently do not use Rust, but we have many tests to keep the quality high. We use code sanitizers, static analysis tools, bounded model checkers, and fuzzy tools (American Fuzzy Lop). From time to time we also consider to use Rust---at least for some optional parts.

You're absolutely right that we need to review these decisions carefully.

I agree that the decision is important. We are currently porting LCDproc and would be happy to help you with your migration, too. One drawback of Elektra is that we do not yet have released 1.0, so API changes are possible. But we certainly can find a migration path for you without breaking anything. (You could use a higher-level API we keep compatible beyond 1.0).

If you do not want to use Elektra, I nevertheless can review your config lib API so that you do not have to learn the same lessons Elektra had to learn the hard way ;)

markus2330 commented Aug 20, 2017

Maybe you want to consider going even a step further: Instead providing yet another config library, you/we could put your parsing code as plugin in Elektra.

We already have a pretty solid i3 config parser, I doubt the change overhead would be worth it for us.

Every other application using Elektra could also use your format, so we love to get new good parsers. With the plugin in place, i3 could keep the same syntax by default. Nevertheless, with Elektra i3 users could also choose their own favorite configuration file format (such as INI, json, XML,..).

If the library is written with GObject (included with GLib)
Then we would definitely not use it 😉

Libelektra does not use GObject in its core (only C, no dep), but it has bindings for GObject and gobject-introspection (next to other bindings such as python, java, lua, ruby, C++).

Elektra also already provides quite some tooling such as command-line tools, graphical editors, and soon web frontends. You should not underestimate the effort to provide such an environment. There is much more involved to it than having a C-library ;) In particular providing proper configuration validation is quite a heavy job: but essential for a good user experience.

Rust

We currently do not use Rust, but we have many tests to keep the quality high. We use code sanitizers, static analysis tools, bounded model checkers, and fuzzy tools (American Fuzzy Lop). From time to time we also consider to use Rust---at least for some optional parts.

You're absolutely right that we need to review these decisions carefully.

I agree that the decision is important. We are currently porting LCDproc and would be happy to help you with your migration, too. One drawback of Elektra is that we do not yet have released 1.0, so API changes are possible. But we certainly can find a migration path for you without breaking anything. (You could use a higher-level API we keep compatible beyond 1.0).

If you do not want to use Elektra, I nevertheless can review your config lib API so that you do not have to learn the same lessons Elektra had to learn the hard way ;)

@SirCmpwn

This comment has been minimized.

Show comment
Hide comment
@SirCmpwn

SirCmpwn Aug 20, 2017

Every other application using Elektra could also use your format, so we love to get new good parsers. With the plugin in place, i3 could keep the same syntax by default. Nevertheless, with Elektra i3 users could also choose their own favorite configuration file format (such as INI, json, XML,..).

This seems like an antifeature to me. Now users can't share config snippets and figuring out configs in bug reports will be a nightmare...

In particular providing proper configuration validation is quite a heavy job

Not... really. This is already implemented in both i3 and sway and isn't particularly tough.

SirCmpwn commented Aug 20, 2017

Every other application using Elektra could also use your format, so we love to get new good parsers. With the plugin in place, i3 could keep the same syntax by default. Nevertheless, with Elektra i3 users could also choose their own favorite configuration file format (such as INI, json, XML,..).

This seems like an antifeature to me. Now users can't share config snippets and figuring out configs in bug reports will be a nightmare...

In particular providing proper configuration validation is quite a heavy job

Not... really. This is already implemented in both i3 and sway and isn't particularly tough.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Aug 20, 2017

This seems like an antifeature to me. Now users can't share config snippets and figuring out configs in bug reports will be a nightmare...

It is just syntax for key/value and you always know how it will be interpreted: You can simply export into other formats, and you can (and should) unify the formats for bug reports. Even better with Elektra you can introspect and exactly see which configuration settings your application will get. (On a key/value basis).

We did not have any problems with it, and there are many benefits for people not knowing a particular syntax. But obviously you can always say other syntaxes are unsupported.

Not... really. This is already implemented in both i3 and sway and isn't particularly tough.

Of course it depends on the goals. If you want the validation to always take place (for tools changing the configuration), across different interfaces (in different programming languages, such as the GUIs, and visudo like functionality), and consider constraints outside your configuration file it gradually gets harder.

markus2330 commented Aug 20, 2017

This seems like an antifeature to me. Now users can't share config snippets and figuring out configs in bug reports will be a nightmare...

It is just syntax for key/value and you always know how it will be interpreted: You can simply export into other formats, and you can (and should) unify the formats for bug reports. Even better with Elektra you can introspect and exactly see which configuration settings your application will get. (On a key/value basis).

We did not have any problems with it, and there are many benefits for people not knowing a particular syntax. But obviously you can always say other syntaxes are unsupported.

Not... really. This is already implemented in both i3 and sway and isn't particularly tough.

Of course it depends on the goals. If you want the validation to always take place (for tools changing the configuration), across different interfaces (in different programming languages, such as the GUIs, and visudo like functionality), and consider constraints outside your configuration file it gradually gets harder.

@Piankero

This comment has been minimized.

Show comment
Hide comment
@Piankero

Piankero Jun 27, 2018

Dear i3 community,

I would be interested in refactoring i3 in the following way:

  • move the parser out to an Elektra plugin (either as extra
    library to be only used by Elektra+i3 or directly move
    the source into Elektra)
  • patch i3 to use Elektra (if the source code is moved to
    Elektra). The advantage of patching i3 is that configuration
    could be generated/transformed with all plugins Elektra has
    (e.g. from encrypted configuration, files from git or
    fetched via curl).
  • write an external specification for the i3 configuration
    so that tooling like the Web UI
    (http://webui.libelektra.org:33334 for a Demo)
    could validate configuration before it is written.

With my proposal this issue could be closed: Elektra's APIs
and bindings would allow anyone to reconfigure i3.
The file format would stay exactly the same because
the same parser would be used.

Would you accept such a patch?
Are you interested?

Piankero commented Jun 27, 2018

Dear i3 community,

I would be interested in refactoring i3 in the following way:

  • move the parser out to an Elektra plugin (either as extra
    library to be only used by Elektra+i3 or directly move
    the source into Elektra)
  • patch i3 to use Elektra (if the source code is moved to
    Elektra). The advantage of patching i3 is that configuration
    could be generated/transformed with all plugins Elektra has
    (e.g. from encrypted configuration, files from git or
    fetched via curl).
  • write an external specification for the i3 configuration
    so that tooling like the Web UI
    (http://webui.libelektra.org:33334 for a Demo)
    could validate configuration before it is written.

With my proposal this issue could be closed: Elektra's APIs
and bindings would allow anyone to reconfigure i3.
The file format would stay exactly the same because
the same parser would be used.

Would you accept such a patch?
Are you interested?

@stapelberg

This comment has been minimized.

Show comment
Hide comment
@stapelberg

stapelberg Jun 27, 2018

Member

I haven’t looked at Elektra in detail, but in general I’d like to avoid complexity and external dependencies where possible.

Given the maturity of the i3 parser, I don’t think we’ll do enough changes that keeping two parsers in sync would be too much effort.

Member

stapelberg commented Jun 27, 2018

I haven’t looked at Elektra in detail, but in general I’d like to avoid complexity and external dependencies where possible.

Given the maturity of the i3 parser, I don’t think we’ll do enough changes that keeping two parsers in sync would be too much effort.

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Jun 27, 2018

I haven’t looked at Elektra in detail, but in general I’d like to avoid complexity and external dependencies where possible.

Very understandable. If you cannot benefit of any of its features there is no reason to add the dependency.

Another feature that might be of interest of you: Elektra has notification via plugins, i.e., you could trigger configuration reloads via dbus/zeromq ... In oyranos the notification finally was the reason to keep the dependency.

An alternative to a hard dependency is to add a compile time option to either use Elektra or directly the parser.

Given the maturity of the i3 parser, I don’t think we’ll do enough changes that keeping two parsers in sync would be too much effort.

Yes, I think so, too. Nevertheless, it is a bit smelly to have such a large duplication.

markus2330 commented Jun 27, 2018

I haven’t looked at Elektra in detail, but in general I’d like to avoid complexity and external dependencies where possible.

Very understandable. If you cannot benefit of any of its features there is no reason to add the dependency.

Another feature that might be of interest of you: Elektra has notification via plugins, i.e., you could trigger configuration reloads via dbus/zeromq ... In oyranos the notification finally was the reason to keep the dependency.

An alternative to a hard dependency is to add a compile time option to either use Elektra or directly the parser.

Given the maturity of the i3 parser, I don’t think we’ll do enough changes that keeping two parsers in sync would be too much effort.

Yes, I think so, too. Nevertheless, it is a bit smelly to have such a large duplication.

@stapelberg

This comment has been minimized.

Show comment
Hide comment
@stapelberg

stapelberg Jun 27, 2018

Member

Another feature that might be of interest of you: Elektra has notification via plugins, i.e., you could trigger configuration reloads via dbus/zeromq ... In oyranos the notification finally was the reason to keep the dependency.

We intentionally try to keep our configuration mechanism as simple as possible. See for example #1197 for why we don’t even have an include feature.

An alternative to a hard dependency is to add a compile time option to either use Elektra or directly the parser.

We don’t do optional dependencies in the i3 projects. We’d like i3 to behave exactly the same way on all installations, barring operating system differences.

Member

stapelberg commented Jun 27, 2018

Another feature that might be of interest of you: Elektra has notification via plugins, i.e., you could trigger configuration reloads via dbus/zeromq ... In oyranos the notification finally was the reason to keep the dependency.

We intentionally try to keep our configuration mechanism as simple as possible. See for example #1197 for why we don’t even have an include feature.

An alternative to a hard dependency is to add a compile time option to either use Elektra or directly the parser.

We don’t do optional dependencies in the i3 projects. We’d like i3 to behave exactly the same way on all installations, barring operating system differences.

@alexherbo2

This comment has been minimized.

Show comment
Hide comment
@alexherbo2

alexherbo2 Jun 27, 2018

How about bspwm configuration being controlled by the command-line, so that the config file can be a simple executable?

alexherbo2 commented Jun 27, 2018

How about bspwm configuration being controlled by the command-line, so that the config file can be a simple executable?

@markus2330

This comment has been minimized.

Show comment
Hide comment
@markus2330

markus2330 Jun 27, 2018

We intentionally try to keep our configuration mechanism as simple as possible. See for example #1197 for why we don’t even have an include feature.

Then we have the same goal.

Elektra supports .d directories with an additional plugin and it also does not support include statements.

We don’t do optional dependencies in the i3 projects. We’d like i3 to behave exactly the same way on all installations, barring operating system differences.

Yes, this makes sense.

How about bspwm configuration being controlled by the command-line, so that the config file can be a simple executable?

Elektra supports all settings to be either from config file, command-line or env var (you can specify which setting should be controlled by which source).

I would not recommend executable code in config files, it is the opposite of being simple 😉

markus2330 commented Jun 27, 2018

We intentionally try to keep our configuration mechanism as simple as possible. See for example #1197 for why we don’t even have an include feature.

Then we have the same goal.

Elektra supports .d directories with an additional plugin and it also does not support include statements.

We don’t do optional dependencies in the i3 projects. We’d like i3 to behave exactly the same way on all installations, barring operating system differences.

Yes, this makes sense.

How about bspwm configuration being controlled by the command-line, so that the config file can be a simple executable?

Elektra supports all settings to be either from config file, command-line or env var (you can specify which setting should be controlled by which source).

I would not recommend executable code in config files, it is the opposite of being simple 😉

@acrisci

This comment has been minimized.

Show comment
Hide comment
@acrisci

acrisci Jun 28, 2018

Member

As for sharing the config between the projects, I don't see this happening. Config parsing for utilities and the wm itself seem to have different goals as well. I think this is more appropriate for a different project.

Member

acrisci commented Jun 28, 2018

As for sharing the config between the projects, I don't see this happening. Config parsing for utilities and the wm itself seem to have different goals as well. I think this is more appropriate for a different project.

@acrisci acrisci closed this Jun 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment