Add --sort tree-order flag to list-windows command #1839
Conversation
Introduce a new --sort flag for the list-windows command to control
the order in which windows are displayed. This addresses the need for
different sorting strategies depending on the use case.
What changed:
- Added --sort flag with two possible values:
* tree-order (new default): Windows are listed in tree traversal
order (DFS), matching their visual order in accordion/tiles layout
* app-name: Windows sorted alphabetically by application name,
then by window title (previous default behavior)
Why:
The tree-order sorting provides a more intuitive output that matches
the visual representation of windows in the workspace tree. This is
particularly useful when debugging or inspecting window arrangements,
as the list order corresponds to the actual layout order.
How:
- Added SortOrder enum to ListWindowsCmdArgs with tree-order and
app-name variants
- Modified ListWindowsCommand to apply sorting based on the flag
- tree-order leverages existing allLeafWindowsRecursive traversal
- Updated documentation and generated help text to reflect the new flag
Breaking change: Default sorting changed from app-name to tree-order.
Users who rely on alphabetical sorting should explicitly use
--sort app-name flag.
prawwtocol
force-pushed
the
feature/list-windows-sort
branch
from
67b7aab to
af80dba
Compare
| + | ||
| Possible values: + | ||
| + | ||
| . `tree-order` - (default) Windows are listed in tree traversal order (DFS). This matches the visual order in accordion/tiles layout. |
There was a problem hiding this comment.
Why do you think that tree-order is a better default? (It's a genuine question)
| @@ -1 +1 @@ | |||
| 6.2.1 | |||
| 6.2.1 No newline at end of file | |||
There was a problem hiding this comment.
Please don't touch unrelated files for no reasons
| "--app-bundle-id": singleValueSubArgParser(\.filteringOptions.appIdFilter, "<app-bundle-id>") { $0 }, | ||
|
|
||
| // Sorting flag | ||
| "--sort": singleValueSubArgParser(\._sortOrder, "<sort-order>") { SortOrder(rawValue: $0) }, |
There was a problem hiding this comment.
--sort flag should conflict with --count flag
| } else { | ||
| var _list: [(window: Window, title: String)] = [] // todo cleanup | ||
| for window in windows { | ||
| _list.append((window, try await window.title)) |
There was a problem hiding this comment.
Please link the appropriate issue in the commit message: #491
| switch args.sortOrder { | ||
| case .appName: | ||
| _list = _list.sortedBy([{ $0.window.app.name ?? "" }, \.title]) | ||
| case .treeOrder: | ||
| break // Already in tree order from allLeafWindowsRecursive | ||
| } |
There was a problem hiding this comment.
Code formatting, the CI doesn't pass
| "--app-bundle-id": singleValueSubArgParser(\.filteringOptions.appIdFilter, "<app-bundle-id>") { $0 }, | ||
|
|
||
| // Sorting flag | ||
| "--sort": singleValueSubArgParser(\._sortOrder, "<sort-order>") { SortOrder(rawValue: $0) }, |
There was a problem hiding this comment.
I'd prefer to call the flag --sort-by
| public var _sortOrder: SortOrder? = nil | ||
| public var sortOrder: SortOrder { _sortOrder ?? .treeOrder } |
There was a problem hiding this comment.
| public var _sortOrder: SortOrder? = nil | |
| public var sortOrder: SortOrder { _sortOrder ?? .treeOrder } | |
| public var sortOrder: SortOrder = .treeOrder |
The second variable seems redundant
| "--app-bundle-id": singleValueSubArgParser(\.filteringOptions.appIdFilter, "<app-bundle-id>") { $0 }, | ||
|
|
||
| // Sorting flag | ||
| "--sort": singleValueSubArgParser(\._sortOrder, "<sort-order>") { SortOrder(rawValue: $0) }, |
There was a problem hiding this comment.
Please use parseEnum util function as it produces nicer error message in case of parsing error
| [--sort <sort-order>] [--format <output-format>] | ||
| [--count] [--json] | ||
| aerospace list-windows [-h|--help] --all [--format <output-format>] [--count] [--json] | ||
| aerospace list-windows [-h|--help] --all [--sort <sort-order>] [--format <output-format>] [--count] [--json] |
There was a problem hiding this comment.
Please also update grammar/commands-bnf-grammar.txt
| "--app-bundle-id": singleValueSubArgParser(\.filteringOptions.appIdFilter, "<app-bundle-id>") { $0 }, | ||
|
|
||
| // Sorting flag | ||
| "--sort": singleValueSubArgParser(\._sortOrder, "<sort-order>") { SortOrder(rawValue: $0) }, |
There was a problem hiding this comment.
Similar to --workspace and --monitor, <sort-order> should be not a scalar value but a space-separated array. Otherwise, the current .sortedBy([{ $0.window.app.name ?? "" }, \.title]) sorting is not representable
Right now, you call it app-name, but it's just wrong. The flag that represents the current default sorting should be --sort-by app-name window-title
2 participants
PR checklist
Failure to follow the checklist with no apparent reasons will result in silent PR rejection.