Component architecture rewrite by dogamak · Pull Request #20 · dogamak/xcbars

dogamak · 2017-06-28T20:01:07Z

The trait Component is now split into ComponentConfig ans
ComponentState, which allows us more flexibility with the
user facing configuration types. (Eg. accepting a &str and copying it
into a String afterwards)

The ComponentState has now more methods than the previous
Component trait had, which allows for better error messages. The
render method also accepts a cairo::Surface so that components can
render arbitary visuals on the bar.

I'll create this pull request for my own commit. I have no idea if this is a common practice or not. Actually I don't have a clue about anything regarding git workflows, but this seems like a thing that could be a thing. 😕

This is a counterpart of #8. It seems like github uses the same id space for bot pull requests and issues, so I might be a little confused here...

The trait `Component` is now split into `ComponentConfig` ans `ComponentState`, which allows us more flexibility with the user facing configuration types. (Eg. accepting a `&str` and copying it into a `String` afterwards) The `ComponentState` has now more methods than the previous `Component` trait had, which allows for better error messages. The `render` method also accepts a `cairo::Surface` so that components can render arbitary visuals on the bar.

chrisduerr · 2017-06-28T22:00:50Z

Well like this travis seems to automatically check the builds which is nice.
Especially in situation like this where travis actually fails. It seems like it has issues resolving the dependencies.

error[E0432]: unresolved import `component::Component`
 --> src/components/pipe.rs:1:5
  |
1 | use component::Component;
  |     ^^^^^^^^^^^^^^^^^^^^ no `Component` in `component`

dogamak · 2017-06-29T08:01:08Z

Ah, I have only implemented the traits in src/component.rs and not yet ported anything to use them. Travis is expected to fail at this point.

Replace `ItemState` with `ComponentContext` and `Box<ComponentStateExt>`. Adresses #8

Redesing the `Component*` traits so that `ComponentState` doesn't have to implement `Stream`. Also modified `Bar` to use single `Vec` to store all components and created `XcbContext` to bundle XCB related information. `ComponentContext` was also changed drastically. Created `ComponentStateWrapper` to wrap `ComponentState` and the associated stream in such a way that we can preserve full type safety.

dogamak · 2017-07-01T22:09:13Z

Come up with an idea of how stuff should work
Sweat your ass of fighting the compiler that doesn't like your visions
Wohoo it finally compiles
Realize that it's impossible to implement something using this desing
Rinse and repeat

Programming is FUN!

chrisduerr · 2017-07-01T22:21:41Z

At least travis seems to like it more now. What design issues did you run into? Could you resolve them?

dogamak · 2017-07-01T22:33:32Z

Initially I required ComponentState to implement Stream (to deliver updates) for whatever reason, which meant that the stream couldn't be created in a conventional way and returned from create() or updates().

I hope I have resolved the issues successfully and that the current version has managed to improve the type safety while simplifying the interface. (Especially by cutting the number of different traits.) Now ComponentState::create returns a (Self, Self::Stream), which allows to the implementer to reuse the different streams and combinators provided in the futures crate.

Next up is implementing the abstraction layer which provides StringComponent trait for implementing components similar to what we have now.

dogamak · 2017-07-01T23:20:57Z

Also, I need an interface for a component to get the bar properties (e.g. fg/bg colors, height).

Implement `StringComponent` as an abstraction for implementing components as streams of strings.

Did not render *anything* before this commit.

…ng it. This is archieved through `ComponentContainer` wrapper enum and `ComponentContainerExt` trait, which replaces the `ComponentConfigExt` as the way of storing the component config before crating it.

Fix an integer overflow that occurs when rendering centered components.

dogamak · 2017-07-14T15:41:39Z

Argh. Travis fails because of some weird PangoCairo C-binding stuff.
I don't want to deal with this shit now.. or ever really.

chrisduerr · 2017-07-14T15:44:32Z

So dependency issues with the CI and not actual build problems?

It looks like it has nothing to do with Travis. So setting the dependency in Cargo.toml to a fixed commit will probably make this work for now. In the long run the plan is to go with crates.io anyways, right?

dogamak · 2017-07-14T16:32:02Z

I don't think any of our dependencies have updated in the last 2 months, so I think this is debian updating their glib/cairo/pango packages or something.

Also, works on my machine.

chrisduerr · 2017-07-14T16:36:36Z

No I've just tried running cargo clean and removed the lockfile and I ran into the same issue.

Using Archlinux with latest nightly.

dogamak · 2017-07-14T17:01:59Z

Oh. After cargo clean it fails for me too. The reason seems to be this commit, but I don't know how I can resolve this as we don't directly depend on glib.

And with that cargo clean I threw away my working Cargo.lock...

chrisduerr · 2017-07-14T17:05:14Z

Well that is kinda unfortunate. :P
This would probably require looking through the dependencies and checking what depends on dlib. It's probably the pangocairo stuff I'd say? Just trying a few different commits might help?

Unfortunately the Cargo.lock is not in git so all branches broke for me now.

dogamak · 2017-07-14T17:13:10Z

Here's your dependency graph for reference. What do you mean by "trying a few different commits"?

chrisduerr · 2017-07-19T19:43:24Z

It doesn't look like this one restarted the CI.

dogamak · 2017-07-19T19:44:59Z

Well, it might be that we hit some concurrent build quota with these 16 build tasks.

chrisduerr · 2017-07-19T19:47:03Z

Yeah I guess running 6 builds at the same time might be a bit much.

dogamak · 2017-07-19T20:05:23Z

Oh right, this branch still has the old CI configs.

chrisduerr · 2017-08-14T19:42:45Z

What is left to do before this can be merged? Seems like everything is fine with travis.

Split `StringComponent` into `StringComponentConfig` and `StringComponentState` similarly to the `ComponentConfig` and `ComponentState` traits. This is more flexible and I should have done this to begin with. It adds more boilerplate code when implementing trivial components, but that can be simplified with a macro in a future if necessary.

Refactor the components to use the split up `StringComponent` traits.

dogamak · 2017-08-14T20:19:52Z

Ran into problems when trying to implement the NetworkUsage component with the StringComponent trait. Now StringComponent is split up like the ComponentConfig and ComponentState traits.

However there seems to be some issues with tokio usage. All components do not always update independently. This might be caused by prematurely returning Async::NotReady somewhere, but I couldn't find the cause.

The middle slot has BG drawing issues again, maybe I accidentally reversed something while resolving merge conflicts?

chrisduerr · 2017-08-14T20:44:26Z

src/bar.rs

+            if let Some(&(ref ctx, _)) = self.slot_items(Slot::Center).first() {
+                if let Some(old_start) = ctx.position {
+                    self.paint_bg(old_start, old_start + old_width_all)?;
+                }


I can't see an issue but the center problem is probably caused by the changes to redraw_center before this point.

@dogamak I just checked and it seems like the old_width_all is not calculated properly. Is it not possible to get the old width anymore?

I also found the issue that when position changes, but not width, there will be no redraw. Getting the old position works tho. So this is an oversight in my initial implementation.

chrisduerr · 2017-08-14T20:47:27Z

Tokio always is a little troublesome in my experience. Might be a good idea to narrow down the issue and then ask in tokio gitter/irc.

chrisduerr · 2017-08-22T20:21:29Z

One thing I also noticed is that not only the center element old width is wrong, but it also cuts off the element (only small part of it is displayed). So something's not right there.

chrisduerr · 2017-09-05T21:05:01Z

@dogamak Would you accept PRs to this PR? I could look into some of these problems to get this finished already.

dogamak added the architecture label Jun 28, 2017

Port Bar and BarBuilder to use the new traits.

33e1707

Replace `ItemState` with `ComponentContext` and `Box<ComponentStateExt>`. Adresses #8

chrisduerr mentioned this pull request Jul 1, 2017

Change bg color/fg color for component #23

Closed

dogamak added 6 commits July 2, 2017 12:15

Provide BarInfo and Surface extents for the Components.

f9fa91e

Implement StringComponent

b965fc9

Implement `StringComponent` as an abstraction for implementing components as streams of strings.

Make StringComponent public.

25b9d2f

Reimplement components::Text using StringComponent

c31177b

Fix rendering.

ef0c85f

Did not render *anything* before this commit.

Merge branch 'master' into feature-8

115d2c0

dogamak force-pushed the feature-8 branch from cd0956a to 115d2c0 Compare July 12, 2017 17:28

dogamak added 5 commits July 14, 2017 16:17

Provide the config object directly for ComponentState without Box'i…

1b5513e

…ng it. This is archieved through `ComponentContainer` wrapper enum and `ComponentContainerExt` trait, which replaces the `ComponentConfigExt` as the way of storing the component config before crating it.

Implement components::Pipe using StringComponent.

b04b744

Fix an integer overflow

27d91dd

Fix an integer overflow that occurs when rendering centered components.

Fix prematurely returning Async::NotReady and blocking.

fc425bd

Implement components::Pipe using StringComponent

acd69f7

dogamak closed this Jul 19, 2017

dogamak reopened this Jul 19, 2017

dogamak added 15 commits July 19, 2017 23:20

Merge branch 'master' into feature-8

c57745c

Fix component indexing issue

bc4444c

Fix components being rendered before being ready

7992eb7

Calculate StringComponent width properly

d6c835a

Fix component indexing issue

a85bf73

Fix freezing issue caused by errorneous use of tokio

abf18cd

Clean warnings

31d85ca

Update pango api usage

0b96300

Clear more clippy warnings

794a165

Clear more clippy warnings

d0988b8

Fix rustfmt issues

526d713

Merge branch 'feature-8' of github.com:dogamak/xcbars into feature-8

122a3fc

Fix more rustfmt issues

de74e8c

Merge branch 'master' into feature-8

db16fef

Fix rustfmt issues

8ce40c2

dogamak added 3 commits August 14, 2017 23:05

Uncomment XcbEventStream

580dca7

Refactor the components

55dda6a

Refactor the components to use the split up `StringComponent` traits.

chrisduerr reviewed Aug 14, 2017

View reviewed changes

Conversation

dogamak commented Jun 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisduerr commented Jun 28, 2017

Uh oh!

dogamak commented Jun 29, 2017

Uh oh!

dogamak commented Jul 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisduerr commented Jul 1, 2017

Uh oh!

dogamak commented Jul 1, 2017

Uh oh!

dogamak commented Jul 1, 2017

Uh oh!

dogamak commented Jul 14, 2017

Uh oh!

chrisduerr commented Jul 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dogamak commented Jul 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisduerr commented Jul 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dogamak commented Jul 14, 2017

Uh oh!

chrisduerr commented Jul 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dogamak commented Jul 14, 2017

Uh oh!

chrisduerr commented Jul 19, 2017

Uh oh!

dogamak commented Jul 19, 2017

Uh oh!

chrisduerr commented Jul 19, 2017

Uh oh!

dogamak commented Jul 19, 2017

Uh oh!

chrisduerr commented Aug 14, 2017

Uh oh!

dogamak commented Aug 14, 2017

Uh oh!

chrisduerr Aug 14, 2017

Choose a reason for hiding this comment

Uh oh!

chrisduerr Aug 17, 2017

Choose a reason for hiding this comment

Uh oh!

chrisduerr Aug 17, 2017

Choose a reason for hiding this comment

Uh oh!

chrisduerr commented Aug 14, 2017

Uh oh!

chrisduerr commented Aug 22, 2017

Uh oh!

chrisduerr commented Sep 5, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dogamak commented Jun 28, 2017 •

edited

Loading

dogamak commented Jul 1, 2017 •

edited

Loading

chrisduerr commented Jul 14, 2017 •

edited

Loading

dogamak commented Jul 14, 2017 •

edited

Loading

chrisduerr commented Jul 14, 2017 •

edited

Loading

chrisduerr commented Jul 14, 2017 •

edited

Loading