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

Expose structure of ProcessConfig from internal module. #62

Closed
mpickering opened this issue Dec 23, 2022 · 4 comments · Fixed by #63
Closed

Expose structure of ProcessConfig from internal module. #62

mpickering opened this issue Dec 23, 2022 · 4 comments · Fixed by #63

Comments

@mpickering
Copy link

I wanted to write a wrapper of runProcess which would print out the command which was going to run before running it. However, the only way I have display a ProcessConfig is using the Show instance which prints out a lot more information (including full environment).

Ideally it would be good to expose the structure of ProcessConfig via the internal module (so I can write the function myself).

@snoyberg
Copy link
Member

I don't see a problem with that. @tomjaguarpaw any thoughts?

@tomjaguarpaw
Copy link
Collaborator

I'll defer to whatever is your preference, Michael.

I personally don't know immediately how to satisfy the request whilst being sure I'm not violating a desirable invariant or abstraction layer of this library, so it would be very low down my priority queue of Haskell-things-to-work-on.

That said I won't stand in you and @mpickering's way if you want to put together a change that tackles this issue. I'm also open to further discussion about it in case I've missed a reason why it's not easy enough for users to work around (for example by defining a wrapper type for ProcessConfig).

@tomjaguarpaw
Copy link
Collaborator

Oh, actually I think I misunderstood the request. I'm in support of exposing any internals from modules named .Internal as long as users know that they can't rely on their behavior or stability.

So I don't see a problem with this either.

mmhat added a commit to mmhat/typed-process that referenced this issue Feb 21, 2023
mmhat added a commit to mmhat/typed-process that referenced this issue Feb 26, 2023
@tomjaguarpaw
Copy link
Collaborator

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 a pull request may close this issue.

3 participants