0

I want to loop through the menu's options. However, it stops at the first option, since the select without "default:" is blocking and it does not know more options will appear dynamically.

Bellow is the broken code:

package main

import (
    "bytes"
    "fmt"
    "io/ioutil"
    "log"
    "os/exec"
    "strings"
    "time"

    "github.com/getlantern/systray"
    "gopkg.in/yaml.v3"
)

var menuItensPtr []*systray.MenuItem
var config map[string]string
var commands []string

func main() {
    config = readconfig()
    systray.Run(onReady, onExit)
}

func onReady() {
    systray.SetIcon(getIcon("assets/menu.ico"))
    menuItensPtr = make([]*systray.MenuItem,0)
    commands = make([]string,0)
    for k, v := range config {
        menuItemPtr := systray.AddMenuItem(k, k)
        menuItensPtr = append(menuItensPtr, menuItemPtr)
        commands = append(commands, v)
   }
   systray.AddSeparator()
    // mQuit := systray.AddMenuItem("Quit", "Quits this app")
    go func() {
        for {
            systray.SetTitle("My tray menu")
            systray.SetTooltip("https://github.com/evandrojr/my-tray-menu")
            time.Sleep(1 * time.Second)
        }
    }()

    go func() {
        for{
            for i, menuItenPtr := range menuItensPtr {
                

          select { 
/// EXECUTION GETS STUCK HERE!!!!!!!
                case<-menuItenPtr.ClickedCh:
                    execute(commands[i])
                }
            }   
            // select {
            // case <-mQuit.ClickedCh:
            //  systray.Quit()
            //  return
            // // default:
            // }
        }
    }()
}

func onExit() {
    // Cleaning stuff will go here. 
}

func getIcon(s string) []byte {
    b, err := ioutil.ReadFile(s)
    if err != nil {
        fmt.Print(err)
    }
    return b
}

func execute(commands string){
    command_array:= strings.Split(commands, " ")
    command:="" 
    command, command_array = command_array[0], command_array[1:]
    cmd := exec.Command(command, command_array ...)
    var out bytes.Buffer
    cmd.Stdout = &out
    err := cmd.Run()
    if err != nil {
        log.Fatal(err)
    }
    // fmt.Printf("Output %s\n", out.String())
}

func readconfig()  map[string]string{
    yfile, err := ioutil.ReadFile("my-tray-menu.yaml")
    if err != nil {
         log.Fatal(err)
    }
    data := make(map[string]string)
    err2 := yaml.Unmarshal(yfile, &data)
    if err2 != nil {
         log.Fatal(err2)
    }
    for k, v := range data {
         fmt.Printf("%s -> %s\n", k, v)
    }
    return data
}


Bellow is the ugly workaround that works:

package main

import (
    "bytes"
    "fmt"
    "io/ioutil"
    "log"
    "os"
    "os/exec"
    "path/filepath"
    "strings"
    "time"

    "github.com/getlantern/systray"
    "gopkg.in/yaml.v3"
)

var menuItensPtr []*systray.MenuItem
var config map[string]string
var commands []string
var labels []string
var programPath string

func main() {
    setProgramPath()
    config = readconfig()
    time.Sleep(1 * time.Second)
    systray.Run(onReady, onExit)
}

func onReady() {
    systray.SetIcon(getIcon(filepath.Join(programPath,"assets/menu.ico")))
    menuItensPtr = make([]*systray.MenuItem, 0)
    i := 0
    op0 := systray.AddMenuItem(labels[i], commands[i])
    i++
    op1 := systray.AddMenuItem(labels[i], commands[i])
    i++
    op2 := systray.AddMenuItem(labels[i], commands[i])
    i++
    op3 := systray.AddMenuItem(labels[i], commands[i])
    i++

    systray.AddSeparator()
    mQuit := systray.AddMenuItem("Quit", "Quits this app")
    go func() {
        for {
            systray.SetTitle("My tray menu")
            systray.SetTooltip("https://github.com/evandrojr/my-tray-menu")
            time.Sleep(1 * time.Second)
        }
    }()

    go func() {
        for {
            select {
// HERE DOES NOT GET STUCK!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
            case <-op0.ClickedCh:
                execute(commands[0])
            case <-op1.ClickedCh:
                execute(commands[1])
            case <-op2.ClickedCh:
                execute(commands[2])
            case <-op3.ClickedCh:
                execute(commands[3])
            case <-mQuit.ClickedCh:
                systray.Quit()
                return
            }
        }
    }()
}

func onExit() {
    // Cleaning stuff will go here.
}

func getIcon(s string) []byte {
    b, err := ioutil.ReadFile(s)
    if err != nil {
        fmt.Print(err)
    }
    return b
}

func setProgramPath(){
    ex, err := os.Executable()
    if err != nil {
        panic(err)
    }
    programPath = filepath.Dir(ex)
    if err != nil {
        fmt.Println(err)
        os.Exit(1)
    }
}

func execute(commands string) {
    command_array := strings.Split(commands, " ")
    command := ""
    command, command_array = command_array[0], command_array[1:]
    cmd := exec.Command(command, command_array...)
    var out bytes.Buffer
    cmd.Stdout = &out
    err := cmd.Run()
    if err != nil {
        log.Fatal(err)
    }
    fmt.Printf("Output %s\n", out.String())
}

func readconfig() map[string]string {
    yfile, err := ioutil.ReadFile(filepath.Join(programPath,"my-tray-menu.yaml"))
    if err != nil {
        log.Fatal(err)
    }

    data := make(map[string]string)
    err2 := yaml.Unmarshal(yfile, &data)
    if err2 != nil {
        log.Fatal(err2)
    }

    labels = make([]string, 0)
    commands = make([]string, 0)

    for k, v := range data {
        labels = append(labels, k)
        commands = append(commands, v)
        fmt.Printf("%s -> %s\n", k, v)
    }
    fmt.Print(len(labels))
    return data
}

Full source code here: https://github.com/evandrojr/my-tray-menu

Evandro Jr
  • 123
  • 8
  • 1
    There are two approaches: 1) The "fan in" pattern: spawn a separate goroutine to read from each of the channels and make them send _something_ into a single channel which is then monitored by your select statement. That something can be the channel itself. 2) The `reflect` package has a way to "listen" on a slice of channels. I strongly recommend to consider the first approach first. – kostix Aug 06 '22 at 17:36
  • 1
    To elaborate on the fan-in pattern: as long as you make the receiving side be able to easily determine the source of event, this approach can funnel events from multiple sources into a single consumer without it having to know about all that sources itself. – kostix Aug 06 '22 at 17:39
  • Hi @kostix thanks, I like the "fan in" pattern. I have seen it here: https://stackoverflow.com/questions/19992334/how-to-listen-to-n-channels-dynamic-select-statement I just don't know how to apply to my situation. I don't use Go for my daily work. I use it for tiny projects like that. I have made the same project in Kotlin and it was really simple. To be honest I didn't manage to pop up the menu with the left mouse button, only the right button. Then I decided to give it a go to Go. Learning hurts, but I enjoy it, lol. I would be very glad if you could show me a working example. Thanks! :) – Evandro Jr Aug 06 '22 at 18:15
  • 1
    Rob pike has a great talk about multiplexing in go: https://www.youtube.com/watch?v=f6kdp27TYZs He covers the fan-in and fan-out patterns – John Aug 06 '22 at 21:23
  • Hi @John, great video Rob Pike's video! I wish I had seen it before hitting my head against the wall. – Evandro Jr Aug 07 '22 at 23:27

1 Answers1

1

select "chooses which of a set of possible send or receive operations will proceed". The spec sets out how this choice is made:

If one or more of the communications can proceed, a single one that can proceed is chosen via a uniform pseudo-random selection. Otherwise, if there is a default case, that case is chosen. If there is no default case, the "select" statement blocks until at least one of the communications can proceed.

Your working example:

select {
    case <-op0.ClickedCh:
        execute(commands[0])
    case <-op1.ClickedCh:
        execute(commands[1])
    // ...
}

uses select successfully to choose between one of the offered options. However if you pass a single option e.g.

select { 
    case<-menuItenPtr.ClickedCh:
        execute(commands[i])
    }
}  

The select will block until <-menuItenPtr.ClickedCh is ready to proceed (e.g. something is received). This is effectively the same as not using a select:

<-menuItenPtr.ClickedCh:
execute(commands[i])

The result you were expecting can be achieved by providing a default option:

select { 
    case<-menuItenPtr.ClickedCh:
        execute(commands[i])
    }
    default:
}  

As per the quote from the spec above the default option will be chosen if none of the other options can proceed. While this may work it's not a very good solution because you effectively end up with:

for {
   // Check if event happened (not blocking)           
}

This will tie up CPU time unnecessarily as it continually loops checking for events. A better solution would be to start a goroutine to monitor each channel:

for i, menuItenPtr := range menuItensPtr {
    go func(c chan struct{}, cmd string) {
        for range c { execute(cmd) }
    }(menuItenPtr.ClickedCh, commands[i])
}
// Start another goroutine to handle quit

The above will probably work but does lead to the possibility that execute will be called concurrently (which might cause issues if your code is not threadsafe). One way around this is to use the "fan in" pattern (as suggested by @kostix and in the Rob Pike video suggested by @John); something like:

cmdChan := make(chan int)

for i, menuItenPtr := range menuItensPtr {
    go func(c chan struct{}, cmd string) {
        for range c { cmdChan <- cmd }
    }(menuItenPtr.ClickedCh, commands[i])
}

go func() {
    for {
        select { 
            case cmd := <- cmdChan:
                execute(cmd) // Handle command        
           case <-mQuit.ClickedCh:
                systray.Quit()
                return
        }
    }
}()

note: all code above entered directly into the question so please treat as pseudo code!

Brits
  • 14,829
  • 2
  • 18
  • 31
  • 1
    I had neglected to include a loop in the last example; fixed that now so it should process multiple messages. The first approach should work (but I would not advice it and tight loops can have unexpected side effects); unfortunately the link you provided is broken. – Brits Aug 07 '22 at 19:30
  • Since it cannot be edited after 5 minutes I've fixed the link and posted again the comment: Hi @Brits, many thanks for your answer. The second approach “a goroutine to monitor each channel” so far is the only one that worked for me. I have posted it here: https://github.com/evandrojr/my-tray-menu/blob/dynamic_channels_brits/main.go The first option with default value does not work for me (I also have tried before) and the “fan in” solution only executes for the 1st time. – Evandro Jr Aug 07 '22 at 22:49
  • 1
    I'm assuming that, as you accepted the answer, the amended "fan in" option worked for you (sorry the amended comment confuses things a bit because it now appears after my comment :-) ). – Brits Aug 07 '22 at 22:53
  • 1
    Approaches 2 and 3 are working now, many thanks again @Brits! I guess one of the side effects of a tight loop is not allowing enough time to listen for IO changes. If you are really curious about it give yourself a try. But it is not really necessary. I am ashamed of myself for not seeing your mistake. I guess I was anxious to enjoy the sunday. At least I’ve refactored some of my bad code. – Evandro Jr Aug 07 '22 at 23:11
  • Hi @Brit Maybe for you the "default:" select option will, work for me it didn't: Distributor ID: Linuxmint Description: Linux Mint 20.2 Release: 20.2 Codename: uma Anyway the "fan in" pattern seems to be the best approach. Should work for everybody and doesn't waste cpu cycles. – Evandro Jr Aug 07 '22 at 23:24
  • Yes - I would not bother going any further with the `default` select option. It's not something you would want to do in a production system as it needlessly wastes CPU cycles. – Brits Aug 07 '22 at 23:39
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/247128/discussion-between-evandro-jr-and-brits). – Evandro Jr Aug 08 '22 at 16:31