search

Ongy / netlink-hs

Netlink communication for Haskell
BSD 3-Clause "New" or "Revised" License
2 stars 7 forks source link

Fix, more autogenerated constants, little bit of doc #3

Closed vdorr closed 6 years ago

vdorr commented 6 years ago

more autogenerated constants to avoid use of magic number, little bit of "documentation" - pointing to autogenerated values (any suggestion how to refer to stuff in Constants.hs appreciated)

Ongy commented 6 years ago

Not really. It would be better if they had proper types that implement some variation of Enum, but I took this architecture over when I continued this project, and the current version is a bit more flexible towards people using magic numbers until the main repo updates.

And once again, I don't think the build fail is your fault, looks like travis is being weird, but I'll let the rerun time out.

Do you need a release on hackage?

vdorr commented 6 years ago

I don't need release now, i only want the changes to be there when it eventually happens.

Enums would be lovely, network package has case for extensibility in its socket option type (https://hackage.haskell.org/package/network-2.6.3.2/docs/Network-Socket.html#g:9), however it would change API significantly. Maybe a good thing. It might be useful even without changing API (just add (fromIntegral . fromEnum)), and it probably does not take much to enhance generator, do you find it worth doing?

Ongy commented 6 years ago

I remember why I didn't do it. The messageType would have to be overloaded (depending on the netlink family connected). This could be done by adding type argument to Header, another one to Packet and propagating it around. Maybe even bind the makeSocketGeneric argument to that and make NetlinkSocket hold the type as well (if this is going to be a breaking release, better fix everything in sight).

I wouldn't object to it. And I don't mind breaking changes much. The reverse dependencies on hackage show only my own project.

But currently I'm at a "don't touch a running system" point, so I'm not likely to invest much time into this change.

Ongy commented 6 years ago

Oh, also we'd have to use custom instances for some of the Typeclasses. Writing them out in the generator could work. But to not silently break things that use the Extended* constructor, the Eq instance has to convert the compared side into Extended* if it's a known type, or suddenly (and without any obvious warning/error) Extended* 12 doesn't match anymore because it became KnownSomething.

vdorr commented 6 years ago

Very well, lets keep it as it is for now, i'm more interested in my little project than in reinventing this library :) You may ping me, if you decide to do some major overhaul, quite likely i'd be willing to help.

vdorr commented 6 years ago

Still, i'm putting implementation of "Known1|Known2|Other Int" trick on my long TODO list, there are gaps in netlink constants anyway (e.g.RTM_*), so custom typeclasses are minor issue (Eq can compare values converted to Ints), documentation value seem significant to me.