gr2m / octokit-plugin-create-pull-request

Octokit plugin to create a pull request with multiple file changes
MIT License
104 stars 28 forks source link

Prompts gets eaten #126

Closed stefanbuck closed 1 year ago

stefanbuck commented 1 year ago

I'm attempting to wrap this library in a CLI. Thanks to its well-designed API, I made really good progress. However I encountered a problem and solving this would require a change in the library itself.

The idea is to wrap all changes.files values in a function to allow the CLI to prompt a confirmation dialog for each diff. This works if there is only one file change per diff and users of the CLI can accept or reject individual changes.

But if you have multiple files this doesn't work and only the last prompt becomes active.

The desired behaviour would be

I was able to narrow the problem down to https://github.com/gr2m/octokit-plugin-create-pull-request/blob/main/src/create-tree.ts#L19 The Promise.all will resolve all promises at the same time which seems to not work well with stdin. I can see why this was implemented in the first place like that to allow parallel execution of all the promises to speed up the overall process.

The potential fix would be to rewrite this line (or make it somehow configurable) to use a for-of loop instead of Promise.all

 for (const path of Object.keys(changes.files)) {

along with some modifications to the various return calls.

As said, making this the default behaviour would change the performance characteristics of this library. Putting this behind a flag should be possible but having two code paths for this feels not ideal. Maybe there is a third option I'm not aware of.

gr2m commented 1 year ago

changes.files values in a function to allow the CLI to prompt a confirmation dialog for each diff

sounds like a great change. I don't mind having files changed one-by-one. In most common use cases, mutating requests should be throttled anyway.

github-actions[bot] commented 1 year ago

:tada: This issue has been resolved in version 4.2.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket: