Skip to content

feat: add list server command - #2314

Merged
CatsDeservePets merged 10 commits into
gokcehan:masterfrom
CatsDeservePets:clients
Dec 21, 2025
Merged

feat: add list server command#2314
CatsDeservePets merged 10 commits into
gokcehan:masterfrom
CatsDeservePets:clients

Conversation

@CatsDeservePets

@CatsDeservePets CatsDeservePets commented Dec 18, 2025

Copy link
Copy Markdown
Collaborator

This PR adds a new server command list which lists all clients currently connected to the server.

@CatsDeservePets

CatsDeservePets commented Dec 18, 2025

Copy link
Copy Markdown
Collaborator Author

Any thoughts on this, @joelim-work?
Until now, I was using a custom command to show the current PID. Being able to quickly view them all seems useful for debugging.

One could also query jumps to extract the current path for each PID and print them next to it (it works but results in rather ugly code).
Screenshot 2025-12-18 at 10 39 37β€―AM

@joelim-work

Copy link
Copy Markdown
Contributor

What is the practical use case of this exactly? If you need to know the PID of an instance, you can display it in the ruler:

PID={{env "id"}}

As for listing out all the clients, I don't think the server currently has a way of exposing this. I suppose it is fine to add a server command for this, but you could probably achieve the same thing using ps/pidof/pgrep.

@CatsDeservePets

Copy link
Copy Markdown
Collaborator Author

What is the practical use case of this exactly? If you need to know the PID of an instance, you can display it in the ruler:

PID={{env "id"}}

As for listing out all the clients, I don't think the server currently has a way of exposing this. I suppose it is fine to add a server command for this, but you could probably achieve the same thing using ps/pidof/pgrep.

It about convenience.
Currently, I have this in my config to get the current id:

cmd id &{{
	lf -remote "send $id echo ${id}"
}}

Like I said, my use-case is basically debugging remote commands. Using ps or pgrep does not differentiate between clients that are connect or not connected to the server.

I do admit, this is a rather niche use case for sure (and so are the profiling flags).

Also having a client command instead of just a server command allows showing the list inside this menu right inside lf with a nice little indicator for the current $PID.

Again, I completely get the argument that this might be a specialised, uncommon workflow, that is why I was asking for your input.

@joelim-work

Copy link
Copy Markdown
Contributor

OK I thought about this a bit more.

Regarding convenience, I would argue that displaying it in the ruler is even more convenient than the using a custom command since it is shown automatically without having to type :id<enter>. The only downside is that it adds a small amount of clutter, but at this point I don't consider you to be a regular user any more, and it is somewhat expected that you would be able to manage your configurations separately for development and everyday use.

Regarding listing out all clients, using ps and similar programs was just a suggestion, but I agree that it is not a perfect solution. For instance, it will show instances started as lf -single, since they are running but not connected to the server. Therefore, I think it makes sense to add a server command to complement the existing conn/drop commands, but I would like the following changes:

  • The command should be called list so that it is consistent with all of the other server commands, which are verbs and words that are short.
  • The code should be moved to just after the case for drop. I prefer to keep some kind of reasonable order when adding things to a list, instead of just adding it to the bottom.
diff --git a/server.go b/server.go
index 0055656..9deae0a 100644
--- a/server.go
+++ b/server.go
@@ -6,6 +6,7 @@ import (
 	"log"
 	"net"
 	"os"
+	"sort"
 	"strconv"
 )
 
@@ -106,6 +107,15 @@ Loop:
 			} else {
 				echoerr(c, "listen: drop: requires a client id")
 			}
+		case "list":
+			ids := make([]int, 0, len(gConnList))
+			for id := range gConnList {
+				ids = append(ids, id)
+			}
+			sort.Ints(ids)
+			for _, id := range ids {
+				fmt.Fprintln(c, id)
+			}
 		case "send":
 			if rest != "" {
 				word2, rest2 := splitWord(rest)

As for the clients command, I am still not convinced that it needs to be added as a built-in command, for the following reasons:

  • It is a niche use case, for debugging purposes which I do not expect regular users will need.
  • It does not encourage modularity, which involves using configuration to build functionality by joining building blocks (such as the new server command) together.
  • The implementation is hardcoded and not customizable, for instance it does not support using a different indicator other than >, or display colors.

Instead, it should be scripted as a custom command in user configuration like this:

cmd clients %{{
    lf -remote list | while read -r client; do
        if [ "$client" = "$id" ]; then
            printf '[%s] ' "$client"
        else
            printf '%s ' "$client"
        fi
    done
}}

I am aware that using echo or % commands can only display a single line of output, which is probably why you wanted to use the menu, although in this case it is used as multi-line output rather than an actual interactive menu. Currently the menu is only used to display keybindings, marks and completions - extending it to be used for other purposes is something that should be considered separately under #84, but I don't have any plans for it right now.

@CatsDeservePets

CatsDeservePets commented Dec 21, 2025

Copy link
Copy Markdown
Collaborator Author

The only downside is that it adds a small amount of clutter, but at this point I don't consider you to be a regular user any more, and it is somewhat expected that you would be able to manage your configurations separately for development and everyday use.

Fair point.

  • The command should be called list so that it is consistent with all of the other server commands, which are verbs and words that are short.

Done.

  • The code should be moved to just after the case for drop. I prefer to keep some kind of reasonable order when adding things to a list, instead of just adding it to the bottom.

Fair point as well. I only thought about the placement in eval.go, not server.go.
I was unsure about the documentation order. Despite being related to conn and drop, I decided to put list after query and before quit, leaving conn and drop listed separately as internal commands.

As for the clients command, I am still not convinced that it needs to be added as a built-in command, for the following reasons:

  • It is a niche use case, for debugging purposes which I do not expect regular users will need.
  • It does not encourage modularity, which involves using configuration to build functionality by joining building blocks (such as the new server command) together.
  • The implementation is hardcoded and not customizable, for instance it does not support using a different indicator other than >, or display colors.

I am also fine with this.

@CatsDeservePets CatsDeservePets changed the title feat: add clients command feat: add list server command Dec 21, 2025
@CatsDeservePets
CatsDeservePets marked this pull request as ready for review December 21, 2025 04:30
@CatsDeservePets
CatsDeservePets merged commit 59a27bb into gokcehan:master Dec 21, 2025
32 checks passed
@CatsDeservePets CatsDeservePets added the new Pull requests that add new behavior label Dec 22, 2025
@CatsDeservePets CatsDeservePets added this to the r41 milestone Dec 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new Pull requests that add new behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants