RFC - Revert "cmds/core/gosh: replace go-prompt with bubbline"#2666
RFC - Revert "cmds/core/gosh: replace go-prompt with bubbline"#266610000TB wants to merge 6 commits into
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2666 +/- ##
==========================================
+ Coverage 75.24% 75.32% +0.08%
==========================================
Files 413 413
Lines 41943 41889 -54
==========================================
- Hits 31560 31554 -6
+ Misses 10383 10335 -48
☔ View full report in Codecov by Sentry. |
|
Duplicate to #2606 (comment), looking for author replies |
|
|
Is there a significant binary size different due to bubbline?
…On Fri, Apr 28, 2023, 00:22 Marvin Drees ***@***.***> wrote:
Duplicate to #2606 (comment)
<#2606 (comment)>,
looking for author replies
#2606 (comment)
<#2606 (comment)>
—
Reply to this email directly, view it on GitHub
<#2666 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPG3EWJ4CC74UJI7VJV7DLXDNV3VANCNFSM6AAAAAAXOLFN4U>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
Thanks for getting back to me Marvin. Left my clarification in your original PR re: size. can't import latest u-root atm, don't know size impact yet |
|
In addition, Google has been following an upstream first approach, where we strives to put every golang cmd / pkg we develop for Google, into u-root as much as possible. Making it harder for us to ingest u-root would disincentivize people from contributing to u-root where possible, and instead check them into internal only repos. |
|
ping |
This reverts commit fd0aca2. Signed-off-by: David Hu <xuehaohu@google.com>
10000TB
force-pushed
the
revert-2606-gosh-bubbline
branch
from
37a37a7 to
5d86880
Compare
|
I see your point. I'm also in favor of having less dependencies. The issue right now is the following:
Using bubbline:
Ideal solution: Write tab completer from scratch using only the standard library. So using either library will upset some users. Bubbline upsets Google, go-prompt upsets everyone else (and probably Google as well as it's still somewhat broken). If someone has the time to implement the completer from scratch then I'm more than happy to review a PR for that but I'm swamped in work so I can't take that on myself. |
|
I think bubbline is fine - I don't yet see the amount of work David is
attaching here. I also think such Google-oddities shouldn't spill into the
open source projects constraints without us at least having tried.
I'm asking David to try the import, and giving a hand.
…On Mon, May 1, 2023, 23:42 Marvin Drees ***@***.***> wrote:
I see your point. I'm also in favor of having less dependencies. The issue
right now is the following:
Using go-prompt:
- Somewhat broken (leave the term in a weird state, can't handle
edgecases, as several open issues)
- Original repo abandoned -> we needed a fork in order to get fixes in
-> more maintainance
- Less dependencies
Using bubbline:
- Not broken as go-prompt
- Still maintained upstream
- More dependencies -> higher investment for Googles internal use
Ideal solution: Write tab completer from scratch using only the standard
library.
So using either library will upset some users. Bubbline upsets Google,
go-prompt upsets everyone else (and probably Google as well as it's still
somewhat broken). If someone has the time to implement the completer from
scratch then I'm more than happy to review a PR for that but I'm swamped in
work so I can't take that on myself.
—
Reply to this email directly, view it on GitHub
<#2666 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAPG3ET7JXM3JJB4Q2FZ3KDXECUG3ANCNFSM6AAAAAAXOLFN4U>
.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
Ironic that the tab completion is seen as a positive when inside Google we are trying to find a way to disable it. |
5 participants
Reverts #2606
Let's consider switch back to go-prompt, which has more public interests
which can be assumed better maintained, w.r.t reliability and security.
Did we consider submitting the "new features" into go-prompt ?