search

awesome-gocui / gocui

Minimalist Go package aimed at creating Console User Interfaces.
BSD 3-Clause "New" or "Revised" License
350 stars 39 forks source link

Community merges #24

Closed RIscRIpt closed 5 years ago

RIscRIpt commented 5 years ago

Some time ago I merged various branches from various forks. And added some changes. I'm not sure if it is useful. Feel free to reject this pull request.

mjarkk commented 5 years ago

Great!

From first looks i'm fine with merging this in,
Though i want to first look though the code and see what the others think of the changes.

Also can you take a look at the golangcibot suggestions?

glvr182 commented 5 years ago

Legend! I will go through all the code / changes tonight. Thank you so much for this PR!

RIscRIpt commented 5 years ago

@mjarkk, I fixed warnings of golangcibot with the commit c239f2. I suppose panic in this case is acceptable, because those functions must not return errors.

mjarkk commented 5 years ago

The goroutine.go and table.go example projects seems to be broken: 

# goroutine.go
./goroutine.go:52:24: not enough arguments in call to g.SetView
    have (string, number, number, number, number)
    want (string, int, int, int, int, byte)

# table.go
./table.go:37:24: not enough arguments in call to g.SetView
    have (string, int, int, int, int)
    want (string, int, int, int, int, byte)
./table.go:68:24: not enough arguments in call to gocui.NewGui
    have (gocui.OutputMode)
    want (gocui.OutputMode, bool)

Beside that the widgets.go example doesn't show the focus on the buttons.
And the example in doc.go and README.md is now broken.

RIscRIpt commented 5 years ago

I made goroutine.go and table.go examples compiling, but they are broken, because view.SetWritePos is broken. Most likely it is because of this commit: f50da5a5, which adds wide character support. I will try to investigate this issue now and try to resolve it.

Edit: fixed in ed6975a

glvr182 commented 5 years ago

Good to hear that it works now, Ill give it another read in a min. For the admins: Please do not merge it yet, wait for my review

glvr182 commented 5 years ago

@RIscRIpt If you are fine with it, I can merge this today (I want to do it via my command line so I have more control over git)

RIscRIpt commented 5 years ago

@glvr182, this merge is all up to the maintainers of this repository. Unfortunately, I won't have much time in the near future. So if some bugs will arise due to this PR, I cannot guarantee I will have enough time to fix them.

Nevertheless, I'm fine with the merge, as it gives opportunity to create more complex UIs by utilising SetWritePos function.