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

Migrate from gtk2hs to gi-gtk #256

Closed
IvanMalison opened this issue Mar 22, 2018 · 20 comments

Comments

Projects
None yet
6 participants
@IvanMalison
Copy link
Member

commented Mar 22, 2018

HELP WANTED! Choose a module, and change it to use gi-gtk. If you are looking for an example of how to do this, take a look at System.Taffybar.Widget.Windows
Modules that need to be converted:

  • System.Taffybar.Widget
  • System.Taffybar.Widget.Battery
  • System.Taffybar.Widget.CPUMonitor
  • System.Taffybar.Widget.CommandRunner
  • System.Taffybar.Widget.Decorators
  • System.Taffybar.Widget.DiskIOMonitor
  • System.Taffybar.Widget.FSMonitor
  • System.Taffybar.Widget.FreedesktopNotifications
  • System.Taffybar.Widget.Generic.DynamicMenu
  • System.Taffybar.Widget.Generic.Graph
  • System.Taffybar.Widget.Generic.Icon
  • System.Taffybar.Widget.Generic.PollingBar
  • System.Taffybar.Widget.Generic.PollingGraph
  • System.Taffybar.Widget.Generic.PollingLabel
  • System.Taffybar.Widget.Generic.VerticalBar
  • System.Taffybar.Widget.GitHubNotifications
  • System.Taffybar.Widget.Layout
  • System.Taffybar.Widget.MPRIS2
  • System.Taffybar.Widget.NetMonitor
  • System.Taffybar.Widget.Generic.ChannelGraph
  • System.Taffybar.Widget.NetworkGraph
  • System.Taffybar.Widget.SNITray
  • System.Taffybar.Widget.SimpleClock
  • System.Taffybar.Widget.Systray
  • System.Taffybar.Widget.Text.CPUMonitor
  • System.Taffybar.Widget.Text.MemoryMonitor
  • System.Taffybar.Widget.Text.NetworkMonitor
  • System.Taffybar.Widget.Util
  • System.Taffybar.Widget.Volume
  • System.Taffybar.Widget.Weather
  • System.Taffybar.Widget.Windows
  • System.Taffybar.Widget.Workspaces
  • System.Taffybar.Widget.XDGMenu.Menu
  • System.Taffybar.Widget.XDGMenu.MenuWidget

Original post below:

The consensus of the haskell community seems to be that the generated gtk bindings offered by https://github.com/haskell-gi/haskell-gi should be preferred to the gtk2hs bindings that taffybar currently uses.

I've been aware that we really ought to migrate for some time, but my attempts to solve #109 involve gi-gtk code, and could necessitate migrating taffybar to gi-gtk. Unfortunately, this migration will not be an easy one, since the interfaces to the two libraries are almost completely different. Further complicating the issue is the fact that the migration can't really be done incrementally AFAICT.

I want to start working on this migration immediately, but I want to get the thoughts of taffybar users and contributors about the best way to proceed.

Because incremental work is impossible given how deeply entrenched taffybar's dependency on gtk2hs is, I am considering creating an entirely new repository to house a greenfield implementation of the core functionality of taffybar using gi-gtk instead of gtk2hs. This is not an action I take lightly as it essentially amounts to a fork, which is an approach that I generally view as counterproductive, because it can be harmful to/divisive of the community. I sort suspect that at least initially, I would probably be the only one working on or using such a fork; even if I did eventually manage to attract some other users, it will probably be more of a slow trickle rather than a sudden exodus, and that situation (2 versions causing confusion and division) is obviously less than ideal.

One of the ideas that has been kicking around in my head is that it might be good to separate taffybar into core and contrib repositories/packages. This change would make migrating taffybar into a much more manageable task. The contrib migration would obviously take a little bit longer, but it could be done peicemeal. In this world there would be coexisting gi and gtk2hs versions of taffybar (ideally simply separated by a major version number) that would coexist until the migration is complete.

I guess I don't really have any specific questions, but I'd love to hear peoples thoughts. Is there a way that this could be approached that avoids some of the concerns I have? What do people think about taffybar core vs contrib? Would anyone be willing to help with the migration? Is anyone strongly opposed to migration?

Not really sure who to tag here, so feel free to add anyone:

@travitch @teleshoes @escherdragon @u11gh @sakshamsharma @iamjamestl @LeifW

@travitch

This comment has been minimized.

Copy link
Member

commented Mar 24, 2018

I'm in favor. I looked into it a few times, but the documentation of gi-gtk at the time was such that I wasn't clear on where to start. I don't mind doing it in place, even if it takes a little while. I kind of doubt that many people depend on the particulars of gtk2hs, but I could be wrong.

@teleshoes

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2018

+1 for gi-gtk

BUT i would like to express my strong wish that the project be kept under one roof, with the gi-gtk work in a PR, for all the reasons you've already said. every greenfield rewrite ive been a part of has ended in tears and a worse overall project, even when it really made sense at the start. i dont think switching UI toolkit APIs is clear enough cause for a fork.

also, i dont think an xmonad-style core/contrib split will meaningfully improve development of taffybar, either, and it will be a long, hard road to get there.

@sakshamsharma

This comment has been minimized.

Copy link
Contributor

commented Mar 24, 2018

+1 for moving to things which the community is moving towards.

I've seen some people who have written custom GTK code in their taffybar.hs, and they might be affected by this. Yet, I do not think a fork is a good idea, since that will only divide the overall project development and user-base.

How about this: a branch for this code, and a master branch deprecation warning for a few months. It would be followed by moving master into a legacy branch, and making this branch the master.

@u11gh

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2018

@IvanMalison IvanMalison added this to TODO in 2.0.0 via automation Apr 1, 2018

@IvanMalison IvanMalison self-assigned this Apr 11, 2018

@IvanMalison IvanMalison moved this from TODO to ASSIGNED in 2.0.0 Apr 11, 2018

@IvanMalison IvanMalison removed their assignment Apr 19, 2018

@IvanMalison IvanMalison moved this from ASSIGNED to TODO in 2.0.0 Apr 19, 2018

@IvanMalison

This comment has been minimized.

Copy link
Member Author

commented Apr 24, 2018

Looks like this actually won't make the 2.0 release. Turns out its a shit ton of work. We do have a start though, in that we have Compat/GtkLibs.hs, and we have some code from other repositories (gtk-sni-tray and gtk-strut) that is written with the gi stuff.

This may need to be divided into

a) rewrite the core using gi-gtk, but still have most of the widgets in gtk2hs
b) slowly rewrite old widgets using gi-gtk

@IvanMalison IvanMalison removed this from TODO in 2.0.0 Apr 24, 2018

@IvanMalison IvanMalison added the easy label May 9, 2018

@teleshoes

This comment has been minimized.

Copy link
Contributor

commented May 10, 2018

....easy? :D

@IvanMalison

This comment has been minimized.

Copy link
Member Author

commented May 10, 2018

@teleshoes
Well it is easy at this point, especially since we now have
System.Taffybar.Compat.GtkLibs for converting between gtk2hs and gi-gtk stuff.

I guess I should clarify that while it is easy, it is also a shitton of work.

My plan was to add a checklist for each module/widget that needs to be converted. Converting a single widget would be a pretty reasonable first contribution, I think.

The whole point of the easy label is that I want to increase community involvement in taffybar. There is a ton of stuff that needs to be done, but there aren't very many people actually contributing, especially contributing in a way that isn't just adding something new.

@Tordek

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2018

Hi! This issue looks interesting to me, but I have a few questions regarding implementing it.

The goal is to remove all references to Graphics.UI.Gtk, and the gtk depedency, right?

E.g., System.Taffybar.Widget.Windows is used as an example and marked as done, but it uses import qualified Graphics.UI.Gtk as Gtk2hs, particularly Gtk2hs.escapeMarkup. There's a non-Gtk2hs version in GI.GLib.Functions; it uses Text instead of String, but it's possible to use it, e.g.:

unGtkEscape :: String -> TaffyIO String                                                                                                                    
unGtkEscape s =
    T.unpack <$> markupEscapeText (T.pack s) (fromIntegral $ length s)

truncatedGetMenuLabel :: Int -> X11Window -> TaffyIO String
truncatedGetMenuLabel maxLength w = do
  fullName <- runX11Def "(nameless window)" (getWindowTitle w)
  unGtkEscape $ truncateString maxLength fullName

But then there's the fact that the constructor is windowsNew :: WindowsConfig -> TaffyIO Gtk2hs.Widget; I guess at some point it'll have to change, too?

(Also, if GI works on Text by default as it seems, should we start moving stuff to use Text as well, like getWindowTitle?)

@IvanMalison

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2018

The goal is to remove all references to Graphics.UI.Gtk, and the gtk depedency, right?

yes

There's a non-Gtk2hs version in GI.GLib.Functions; it uses Text instead of String, but it's possible to use it, e.g.:

Yes, I was actually aware of this, but I just sort of ran out of steam when I was trying to do that last part. Figured I (or someone) would get around to it eventually

But then there's the fact that the constructor is windowsNew :: WindowsConfig -> TaffyIO Gtk2hs.Widget; I guess at some point it'll have to change, too?

Yes, but I think that will be one of the last things that gets done.

For now the core interface still takes gtk2hs widgets, so what is retruned by the widget builders needs to be a gtk2hs widget.

I suppose that we could work on migrating the core NOW, and simply use

https://github.com/travitch/taffybar/blob/d59b74d83a09751970c902f726b266c9d71ebbaa/src/System/Taffybar/Compat/GtkLibs.hs#L46

to convert the widgets. I just haven't gotten around to that yet.

(Also, if GI works on Text by default as it seems, should we start moving stuff to use Text as well, like getWindowTitle?)

Yes

@IvanMalison

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2018

Hi! This issue looks interesting to me, but I have a few questions regarding implementing it.

That's great, I need all the help I can get with it. I assume you're not going to take on ALL of it; it's quite a lot of work.

If you do decide to work on a specific component just post in here so that I (and others) are aware and don't do duplicate work.

@Tordek

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2018

Awesome, thanks for the reply.

Ok, so we can make it a "standard" process for each module to:

  1. Convert import qualified Graphics.UI.Gtk as Gtk2hs as import qualified Graphics.UI.Gtk as Gtk2hs (Widget) so we only need it for the type signature.
  2. Rewrite everything as needed to use GI.Gtk
  3. Finish with >>= fromGIWidget

so eventually when the type signature is changed we just have to change a couple of lines.

And that change can be "just" (which is a lot anyway):

  1. change all the signatures and remove >>= fromGIWidget from each widget
  2. have internal code that "expects a Gtk.Widget but works on Gtk2hs.Widget" do the transformation on its own side until it gets rewritten properly.
  3. Eventually rewrite that properly.

If this looks reasonable, I'll start tackling down some modules when I'm able to.

@IvanMalison

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2018

Ok, so we can make it a "standard" process for each module to:

Convert import qualified Graphics.UI.Gtk as Gtk2hs as import qualified Graphics.UI.Gtk(Widget) as Gtk2hs so we only need it for the type signature.
Rewrite everything as needed to use GI.Gtk
Finish with >>= fromGIWidget

Yep, that's what I've been doing, but yeah, good idea to write it down and make it "official".

have internal code that "expects a Gtk.Widget but works on Gtk2hs.Widget" do the transformation on its own side until it gets rewritten properly.

I'm not exactly sure what this is saying:

Are you referencing changing the bar initiation code in Context.hs?

Are you saying we should make it so that you can provide either type of Widget? We would need to wrap the widget types with an ADT, or existential type then, which IMO doesn't really make things any easier.

@Tordek

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2018

that's what I've been doing

I actually missed it because Window was doing widgetSetClass =<< fromGI...; I've changed it to use widgetSetClassGI instead so it's not in the way.

Are you referencing changing the bar initiation code in Context.hs?

Yes, basically. Change the signature for those functions to take a GI.Gtk.Widget and convert it to Gtk2hs.Widget, while all the widgets perform the opposite conversion with toGIWidget. So, sure, in that moment we're just doing a pointless back-and-forth conversion of types, but then we can start removing references to Gtk2hs more directly.

@IvanMalison

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2018

Yes, basically. Change the signature for those functions to take a GI.Gtk.Widget and convert it to Gtk2hs.Widget, while all the widgets perform the opposite conversion with toGIWidget. So, sure, in that moment we're just doing a pointless back-and-forth conversion of types, but then we can start removing references to Gtk2hs more directly.

I don't think there is any reason to do this. The change in Context.hs will actually be relatively easy, it's just that then we will have to switch all widgets (except for the ones that have already been converted) to using toGIWidget.

I'd rather not take the intermediate step -- we should just make the change in Context and Config.

I've changed it to use widgetSetClassGI instead so it's not in the way.

In a local branch? Don't take on too much in any single change. I'd really prefer it if you can, as much as possible, make PRs that only affect a single widget.

I really think the first thing that should be done, if we really want to take this on, is to make the change in Context and SimpleConfig first.

@Tordek

This comment has been minimized.

Copy link
Contributor

commented Jun 30, 2018

I was looking around and ended up finishing the change on Context and SimpleConfig by accident... I'm now just confused by Graphics.UI.GIGtkStrut which seems to be the haskell-gi-approved version but it just has the wrong namespace.

@IvanMalison

This comment has been minimized.

Copy link
Member Author

commented Jun 30, 2018

@Tordek Yeah, that is my own library. Graphics.UI seems like the right place for it. We'll have to leave that for now.

@Tordek

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2018

Going a bit over what needs to be done:

  • src/System/Taffybar/Compat/GtkLibs.hs
    Would be the last, when everything else is done

  • src/System/Taffybar/Widget/Systray.hs
    I guess just gets deleted, since it's deprecated

  • src/System/Taffybar/Widget/SimpleClock.hs
    Depends on a rewrite of attachPopup, from Util, and I don't even know where to begin with windowSetPosition.

  • src/System/Taffybar/Widget/Util.hs

  • src/System/Taffybar/Widget/MPRIS2.hs

  • src/System/Taffybar/Widget/XDGMenu/MenuWidget.hs

  • src/System/Taffybar/Widget/Weather.hs

  • src/System/Taffybar/Widget/Generic/Icon.hs

  • src/System/Taffybar/Widget/Generic/VerticalBar.hs

@IvanMalison

This comment has been minimized.

Copy link
Member Author

commented Jul 2, 2018

@Tordek Not too much left to do then. Looks reasonable.

@Tordek

This comment has been minimized.

Copy link
Contributor

commented Jul 2, 2018

I've hit a couple of roadblocks, particularly System.Taffybar.Widget.Util.attachPopup, I don't know where to get gTypeWindow from. Also, windowSetPosition; there doesn't seem to be a way to do that in GI.Gtk?

So, basically, the only remaining thing is SimpleClock (modulo some issue that's causing a compile error in one of the versions that I have no idea why it happens, but seems to be related to GI.Gtk.set).

@Tordek

This comment has been minimized.

Copy link
Contributor

commented Jul 3, 2018

I think it's done? gTypeWindow was a bit silly involving gobjectType (undefined :: Window) and I wasn't finding windowSetPosition because the docs kept leading me to Gdk.Window instead of Gtk.Window...

Probably can use a ton of cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.