search

awesome-gocui / gocui

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

prettify examples: add main demo screen to select specific examples #117

Open elv-gilles opened 2 years ago

elv-gilles commented 2 years ago

This patch allows to compile all examples in one binary.

A main 'demo' example is added on top allowing to run each of the specific examples - although each example can still be run individually. see the added README

mjarkk commented 2 years ago
image

The screen is not fully visible on a small terminal.

dankox commented 2 years ago

I'm just wondering, why did you use all the _ assignement in the code? It's kinda incosistent as for example the quit() functions, somewhere assigns the g and v to _ and somewhere it's right in the function definition.
Also I see a bunch of Fprintf and other function calls with these types of assignements.

I don't really understand the reason to put these assignements to a function where you don't check the return value, so I'm wondering what was your reason to add them there.
I know for sure that for example Coverity makes a complain if you don't check returned error values (like in case of the Fprintf), but I don't think there is any coverity running in the pipeline, so there shouldn't be problems with it.
So if you could provide the reason, that would be nice. Not that the code is incorrect, I am just confused by that.

elv-gilles commented 2 years ago

I'm just wondering, why did you use all the _ assignement in the code? Not that the code is incorrect, I am just confused by that.

Humm, sorry to be confusing. I'm running my dev environment with warning enabled for unused variables and unhandled errors which I find very useful. ... but if folks in the project don't like it, I'll refrain myself.

dankox commented 2 years ago

Ok... so let's wait for @mjarkk input.

mjarkk commented 2 years ago

I personally don't like the _ assignments but that's just personal preference.

Tough that said they are also out of the scope of this PR so let's not add them.

elv-gilles commented 2 years ago

@mjarkk - I fixed the github workflow and therefore need another approval.

elv-gilles commented 2 years ago

@dankox @mjarkk - is there anything else I can do ? I don't have the power to merge this myself.

dankox commented 2 years ago

I haven't been around for some time and now I see that this is not really merged.

So I will just give here my opinion, but @mjarkk feel free to merge.

I can see that most of the _ were removed from the original code, but I can still see them in the "main" program. The reason why I don't really like it is because this is only for the sake of squiggly lines in the IDE. It doesn't really add anything to the code, or to its safety. It only makes it look a bit messy. If the linter is complaining about such code, it should be fixed by properly checking the error, instead of just putting there _. Because this doesn't really resolve the problem the linter is reporting. It just hides warning message. It's not really a good practise because the problem is still there and people can't find it out easily anymore. Just wanted to say it so it's clear why I don't like it and complain about it :) These are just demo examples, so no biggie ;) but could be harmful in some other bigger apps if used like this.

As for the PR. At first I was a bit hesitant as I like to have possibility to run just one program using go run without any of the select menu thingy when testing something. But this actually allows such run too, so that's good enough for me and will make it easier for people to quickly try different demos. So :+1: :)

elv-gilles commented 2 years ago

Hello @dankox @mjarkk can we merge this, please ?