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

Split input events into separate package #276

Closed
wants to merge 3 commits into from

Conversation

kostmo
Copy link
Contributor

@kostmo kostmo commented Jun 30, 2024

Context: swarm-game/swarm#1978 (comment)

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.

@kostmo kostmo force-pushed the separate-input-package branch from e9f3e10 to e6d258b Compare June 30, 2024 19:18
@jtdaugherty
Copy link
Owner

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
Copy link
Contributor Author

kostmo commented Jun 30, 2024

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.

@kostmo kostmo force-pushed the separate-input-package branch from bfed3cd to e983516 Compare June 30, 2024 20:53
@jtdaugherty
Copy link
Owner

I ended up switching to cabal as a workaround.

Does this mean you have a working build now?

@kostmo
Copy link
Contributor Author

kostmo commented Jun 30, 2024

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
Copy link
Owner

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 haskell/terminfo#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
Copy link
Contributor Author

kostmo commented Jun 30, 2024

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 swarm-game/swarm#1978 (comment) .

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
Copy link
Owner

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 swarm-game/swarm#1978 (comment) .

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!

@jtdaugherty jtdaugherty closed this Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants