search

jtdaugherty / vty

A high-level ncurses alternative written in Haskell
BSD 3-Clause "New" or "Revised" License
320 stars 57 forks source link

Split input events into separate package #276

Closed kostmo closed 4 months ago

kostmo commented 4 months ago

Context: https://github.com/swarm-game/swarm/issues/1978#issuecomment-2197867361

The dependence on the C module (and a certain system library) limits the portability of the vty package in its entirety. In swarm, we have a "headless" server binary that depends on the definitions of keys to parse swarm "programs". Splitting the key definitions out of vty and into vty-inputs would allow us to compile our server binary without extra dependencies.

I acknowledge that this would add an extra step in the release process for vty, but hopefully the change is minor enough to be palatable.

jtdaugherty commented 4 months ago

The dependence on the C module (and a certain system library) limits the portability of the vty package in its entirety.

Can you say more about this? Are you referring to C code in cbits? Which "certain system library"?

kostmo commented 4 months ago

Can you say more about this? Are you referring to C code in cbits? Which "certain system library"?

I believe it was the tinfo library that was causing trouble. This was preventing compilation of vty in an Amazon Linux docker image with stack. I recall coming across #181 while investigating the issue. I ended up switching to cabal as a workaround.

jtdaugherty commented 4 months ago

I ended up switching to cabal as a workaround.

Does this mean you have a working build now?

kostmo commented 4 months ago

I ended up switching to cabal as a workaround.

Does this mean you have a working build now?

Yes, we do, though it means we have closed the door on going back to stack.

Aside from this practical concern, it would be ideal if a datatype representing keyboard keys existed somewhere, such that future libraries could standardize on it instead of all defining their own versions.

jtdaugherty commented 4 months ago

Yes, we do, though it means we have closed the door on going back to stack.

Are you implying that this is undesirable for your project? I gather that you're implying that it is, but I am wanting to understand your situation clearly.

it would be ideal if a datatype representing keyboard keys existed somewhere, such that future libraries could standardize on it instead of all defining their own versions.

I'm not sure where this is coming from. I mean, I read the linked ticket discussion and I read the comment there about defining a type that had a correspondence to Vty's key event types. I wouldn't recommend doing that.

I'm gathering that in the case of the application you're working on, due to a build issue you ran into, it would be convenient to have some data types in vty provided by a ligher-weight package than vty itself. As for the specific patch here, I'm not open to having just the input types in that package because that's too specific. If I were to do this at all, I'd want to have a vty-types package (say) that provided much more than the input event types. However, I'm not quite ready to take that step. When I had encountered the tinfo problem in the past I thought it was a terminfo issue (that you linked to) but apparently it can arise for other reasons (see https://github.com/judah/terminfo/issues/17). Either way, if it has a resolution in a Haskell package, I believe it would be terminfo rather than vty. But I'm not entirely clear on how to fully deal with this issue since it isn't clear to me how much of it relates to the underlying toolchain.

Also, I wanted to mention that it would be helpful to me in the future (in order to help you!) if you could ping me (on a Swarm issue, say) or stop by the repo and open a ticket about the problem you're having before proposing a solution and writing a patch. That way, it's possible I could help you resolve the issue earlier, but barring that, it would help me work together with you on a solution that would work for me and save you time writing patches I might not accept.

kostmo commented 4 months ago

Are you implying that this is undesirable for your project? I gather that you're implying that it is, but I am wanting to understand your situation clearly.

Although I led with this concern when opening the PR, I suppose it's not the primary motivator. I think we've already burned a couple of other bridges precluding a return to stack in our project anyway. Though I figured our past build issue might be useful to you as a data point.

I'm not sure where this is coming from. I mean, I read the linked ticket discussion and I read the comment there about defining a type that had a correspondence to Vty's key event types. I wouldn't recommend doing that.

Yeah, I think I made a bit more of a generalization here than necessary. You've identified the precise issue---we would like to avoid defining our own type that has a correspondence to Vty's key event types. We'd appreciate your perspective if you could chime in directly in response to https://github.com/swarm-game/swarm/issues/1978#issuecomment-2198339127 .

if you could ping me (on a Swarm issue, say) or stop by the repo and open a ticket about the problem you're having before proposing a solution and writing a patch.

Thanks---I figured it would be courteous to check the feasibility of a library split before bothering you with our issue. I don't mind that this particular PR is not accepted, rather I enjoyed the exercise of figuring out how to do a library split.

jtdaugherty commented 4 months ago

Although I led with this concern when opening the PR, I suppose it's not the primary motivator. I think we've already burned a couple of other bridges precluding a return to stack in our project anyway. Though I figured our past build issue might be useful to you as a data point.

Okay, thanks! I don't keep up with stack at all, myself. For projects that I maintain, my hope is that they'll Just Work for both cabal and stack, and for the most part that seems to work out okay.

You've identified the precise issue---we would like to avoid defining our own type that has a correspondence to Vty's key event types. We'd appreciate your perspective if you could chime in directly in response to https://github.com/swarm-game/swarm/issues/1978#issuecomment-2198339127 .

Okay, thanks - will do!

Thanks---I figured it would be courteous to check the feasibility of a library split before bothering you with our issue. I don't mind that this particular PR is not accepted, rather I enjoyed the exercise of figuring out how to do a library split.

Okay, I see, thanks. I forget that sometimes that's the spirit in which patches are submitted. I guess I've had enough experiences where people submit patches that they have already become very attached to, in which case sometimes that puts me in a difficult position of having to decline the patch and hopefully find another workable solution. That's gotten me into the habit of wanting to talk things over first to avoid disappointment and frustration (usually for myself, sometimes for both parties).

I'm going to close this, but I'm going to give this some more thought and take a deeper look at the Swarm discussion and chime in there when I get time - should be in the next couple of days.

Thanks!