YAML and JSON are optional, but enabled by default.

[Siborgium]

I'll push few commits more, but it is pretty much done atm.

[kennylevinsen]

Cool!

Just to clarify, what's the goal for making these optional? Does it make the binary noticably smaller, or compilation speed noticably faster? I recall serde being quite expensive to compile, but don't know if the deserializers are that bad. I doubt runtime performance is affected by this.

BTW, If you're not done, it's a good idea to either use the new "Draft" PR functionality when you make it (you can't convert a PR into a draft later—bloody GitHub), or to name the PR with a WIP/DO NOT MERGE/something prefix while you're still working. Less risk of an accidental early merge.

[Siborgium]

IMO user is unlikely to have two differently formatted configs at the same time. Honestly, there is no real difference. I've done few clean rebuilds, and build times are quite consistent (+- 2s).

cargo build --release --features=default    
Finished release [optimized] target(s) in 6m 39s
.rwxr-xr-x 4.1M siborgium wldash

cargo build --release --no-default-features --features=alsa-widget,pulseaudio-widget,yaml-cfg
Finished release [optimized] target(s) in 6m 31s
.rwxr-xr-x 4.0M siborgium wldash

The goal is simple: it is very easy to plug new config format (e.g. it would take just 12+2 lines of diff to add TOML), and there would be no overhead. Plus, the way it works is not hard-coded, I think it would be possible to make a macro, generating the most of routine copypaste in serdefmt.rs. It works just like before, but it is simpler to make changes. Note, if, say, YAML is disabled, then print-config-yaml is automatically disabled and won't be recognized by wldash.

And, at the very end, I just don't want to have JSON when I already have YAML.

Draft would be nice, I am not familiar with the most of Github functional as I rarely use it. I am going to clean the new code a bit.

Please, tell me what's better in terms of naming: serdefmt or configfmt? And do we really need to call shrink_to_fit in commit 43ad83? As far I can understand, the goal is to clear input buffer between invocations of daemonized wldash, but why free memory? I preserved the way you cleared it, but IMO if it is not critical, it would be better to not free memory.

[Siborgium]

It turns out I wasn't right. While disabling JSON is just matter of 0.1MB, disabling YAML is matter of ~0.3MB.

cargo build --release --no-default-features --features=pulseaudio-widget,alsa-widget,json-cfg
.rwxr-xr-x 3.7M siborgium wldash

By disabling pulseaudio I free another 0.1MB, resulting in 3.6MB instead of 4.1MB. 0.5MB is quite a big difference in my opinion.

[kennylevinsen]

The goal is simple: it is very easy to plug new config format (e.g. it would take just 12+2 lines of diff to add TOML), and there would be no overhead. Plus, the way it works is not hard-coded, I think it would be possible to make a macro, generating the most of routine copypaste in serdefmt.rs. It works just like before, but it is simpler to make changes. Note, if, say, YAML is disabled, then print-config-yaml is automatically disabled and won't be recognized by wldash.

To be fair, I don't really intend on supporting many config formats—the two right now are just because I couldn't make up my mind: YAML is awful, JSON is awful, TOML doesn't work for this config struct. I hate YAML, so I picked JSON initially... But I ended up regretting, and added YAML support as well.

I'll probably deprecate JSON sooner or later. Just didn't want to pull the rug on those that might have written a config.json right away. :)

Draft would be nice, I am not familiar with the most of Github functional as I rarely use it. I am going to clean the new code a bit.

All OK, it was just a heads up for the future.

Please, tell me what's better in terms of naming: serdefmt or configfmt?

Of those two, configfmt. serdefmt doesn't say anything about what it's a format for.

And do we really need to call shrink_to_fit in commit 43ad83?

No, not really. The original assignment to a new String object was just a brainfart in a quick bugfix. No need to shrink.

By disabling pulseaudio I free another 0.1MB, resulting in 3.6MB instead of 4.1MB. 0.5MB is quite a big difference in my opinion.

To nitpick: One is not meant to disable both forms of config. If you disable JSON (which seemed like what you wanted to do), you only save 0.1MB (0.2MB if you also count pulseaudio). That would have been a noticeable chunk... if the binary hadn't been ~4MB.

All in all, the PR looks good. Just tell me when it's done!

[Siborgium]

I do not disable both. Right now the wldash I use is built with ALSA and JSON. The binary size is 3.6MB. Honestly, I have had to reconsider JSON for benchmarks on the internet say JSON is faster to parse. I agree, 0.5MB is not that much, and that's the best case, however the most of the executable binary size should be the statically linked Rust's std.

[Siborgium]

Consider it done. I will be busy soon, and I don't think I'll manage to create another pull request. However, if lucky, I'll create one more, SSE intrinsics should speed it up a bit. But, again, it is unlikely I'll manage to finish it soon, so feel free to merge.

[kennylevinsen]

Sorry for the delay!

I have manually cleaned up the history, rustfmt'd things and merged it to master. I have kept you as author on the rewritten commits to correctly attribute the work to you.

Thanks for your effort!

[Siborgium]

Yeah, thanks! As for the SSE, there is no need to change anything -- compiler already generates proper instructions, no manual vectorization needed.