-
-
Notifications
You must be signed in to change notification settings - Fork 462
Add --sort tree-order flag to list-windows command #1839
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 6.2.1 | ||
| 6.2.1 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,14 @@ struct ListWindowsCommand: Command { | |
| _list.append((window, try await window.title)) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please link the appropriate issue in the commit message: #491
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thank you for creating an issue for me! will do upd: didn't see this issue when I created my PR. just needed this functionality myself lol! |
||
| } | ||
| _list = _list.filter { $0.window.isBound } | ||
| _list = _list.sortedBy([{ $0.window.app.name ?? "" }, \.title]) | ||
|
|
||
| // Sort based on --sort flag (default: tree-order) | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unnecessary comment |
||
| switch args.sortOrder { | ||
| case .appName: | ||
| _list = _list.sortedBy([{ $0.window.app.name ?? "" }, \.title]) | ||
| case .treeOrder: | ||
| break // Already in tree order from allLeafWindowsRecursive | ||
| } | ||
prawwtocol marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| let list = _list.map { AeroObj.window(window: $0.window, title: $0.title) } | ||
| if args.json { | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,9 @@ public struct ListWindowsCmdArgs: CmdArgs { | |||||||
| "--pid": singleValueSubArgParser(\.filteringOptions.pidFilter, "<pid>", Int32.init), | ||||||||
| "--app-bundle-id": singleValueSubArgParser(\.filteringOptions.appIdFilter, "<app-bundle-id>") { $0 }, | ||||||||
|
|
||||||||
| // Sorting flag | ||||||||
| "--sort": singleValueSubArgParser(\._sortOrder, "<sort-order>") { SortOrder(rawValue: $0) }, | ||||||||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to call the flag
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to Right now, you call it |
||||||||
|
|
||||||||
| // Formatting flags | ||||||||
| "--format": formatParser(\._format, for: .window), | ||||||||
| "--count": trueBoolFlag(\.outputOnlyCount), | ||||||||
|
|
@@ -36,6 +39,8 @@ public struct ListWindowsCmdArgs: CmdArgs { | |||||||
| fileprivate var all: Bool = false // ALIAS | ||||||||
|
|
||||||||
| public var filteringOptions = FilteringOptions() | ||||||||
| public var _sortOrder: SortOrder? = nil | ||||||||
| public var sortOrder: SortOrder { _sortOrder ?? .treeOrder } | ||||||||
|
Comment on lines
+42
to
+43
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
The second variable seems redundant |
||||||||
| public var _format: [StringInterToken] = [] | ||||||||
| public var outputOnlyCount: Bool = false | ||||||||
| public var json: Bool = false | ||||||||
|
|
@@ -47,6 +52,11 @@ public struct ListWindowsCmdArgs: CmdArgs { | |||||||
| public var pidFilter: Int32? | ||||||||
| public var appIdFilter: String? | ||||||||
| } | ||||||||
|
|
||||||||
| public enum SortOrder: String, Sendable { | ||||||||
| case treeOrder = "tree-order" | ||||||||
| case appName = "app-name" | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| extension ListWindowsCmdArgs { | ||||||||
|
|
||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,9 +11,10 @@ include::util/man-attributes.adoc[] | |
| // tag::synopsis[] | ||
| aerospace list-windows [-h|--help] (--workspace <workspace>...|--monitor <monitor>...) | ||
| [--monitor <monitor>...] [--workspace <workspace>...] | ||
| [--pid <pid>] [--app-bundle-id <app-bundle-id>] [--format <output-format>] | ||
| [--pid <pid>] [--app-bundle-id <app-bundle-id>] | ||
| [--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] | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please also update
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you very much for your insightful comments! You know much more about this project than me, of course 🤗 |
||
| aerospace list-windows [-h|--help] --focused [--format <output-format>] [--count] [--json] | ||
|
|
||
| // end::synopsis[] | ||
|
|
@@ -55,6 +56,14 @@ Filter results to only print windows that belong to the Application with specifi | |
| + | ||
| Deprecated (but still supported) flag name: `--app-id` | ||
|
|
||
| --sort <sort-order>:: | ||
| Specify the order in which windows are listed. | ||
| + | ||
| Possible values: + | ||
| + | ||
| . `tree-order` - (default) Windows are listed in tree traversal order (DFS). This matches the visual order in accordion/tiles layout. | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you think that
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I failed to address this in my comment notes but one of the usecases is better integration with window bar applications like https://github.com/Jean-Tinland/simple-bar They benefit from displaying open windows in tree-order instead of id-sorted order
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will include that info in further commit descriptions |
||
| . `app-name` - Windows are sorted alphabetically by application name, then by window title. | ||
|
|
||
| --format <output-format>:: Specify output format. See "Output Format" section for more details. | ||
| Incompatible with `--count` | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't touch unrelated files for no reasons
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weird thing my editor did. you're right. sorry!
also, my PR wasnt ready for review when you left these comments, I would've caught some of these (and have fixed the CI already) as not to waste your time. I will mark the PR as draft in the future