Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Copy and send notification when user click "This device" menu #9

Merged
merged 3 commits into from
Oct 15, 2021
Merged

feat: Copy and send notification when user click "This device" menu #9

merged 3 commits into from
Oct 15, 2021

Conversation

codenoid
Copy link
Contributor

No description provided.

@mattn
Copy link
Owner

mattn commented Oct 13, 2021

This possibly makes goroutine leak since this part is called multiple times.

@codenoid
Copy link
Contributor Author

the fix might be a bit complicated for now

main.go Outdated
@@ -131,6 +138,30 @@ func onReady() {
mDisconnect.Enable()
systray.SetIcon(iconOn)
enabled = true

go func(mThisDevice *systray.MenuItem) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this part is passed twice, goroutine is created two.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Let me debug it again

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

building...                                   
running...                                    
ts-called goroutine id: 1634266073 # first loop cycle            
# I made-up error intentionally (dynamically without closing program) on tailscale exec call, this will send "closing" signal to enabledCh
ts-closed goroutine id: 1634266073 # this goroutine close was triggered by enabledCh
# I remove the made-up error
ts-called goroutine id: 1634266103 # The click listener active again after if !enable { enable = true }
^Ccleaning...                                 
see you again~                                

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Screenshot from 2021-10-15 09-57-22

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The groutine part should be moved out of for loop.

Copy link
Contributor Author

@codenoid codenoid Oct 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it like this?

systray.AddSeparator()
mAdminConsole := systray.AddMenuItem("Admin Console...", "")
go func() {
for {
_, ok := <-mAdminConsole.ClickedCh
if !ok {
break
}
openBrowser("https://login.tailscale.com/admin/machines")

Copy link
Contributor Author

@codenoid codenoid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦‍♂️ wrong commit message

@mattn
Copy link
Owner

mattn commented Oct 15, 2021

Probably LGTM. Will look tonight.

@mattn mattn merged commit 20c7e85 into mattn:main Oct 15, 2021
@mattn
Copy link
Owner

mattn commented Oct 15, 2021

Thank you

@codenoid
Copy link
Contributor Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants