From: Iskren Chernev <me@iskren.info>
It is useful to cycle between layout with a single command, without
caring what is the current layout. The command accepts a list of
Locations divided by comma (','). If the current location is not in the
list, the first one is chosen. Otherwise the next in the list (wrapping
around) is taken.
---
src/main.zig | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/src/main.zig b/src/main.zig
index 877d8b2..26b2440 100644
--- a/src/main.zig+++ b/src/main.zig
@@ -64,6 +64,7 @@ const Command = enum {
@"outer-gaps",
gaps,
@"main-location",
+ @"main-location-cycle", @"main-count",
@"main-ratio",
@"width-ratio",
@@ -188,6 +189,34 @@ const Output = struct {
return;
};
},
+ .@"main-location-cycle" => {+ var loc_cnt: u8 = 0;+ // support a maximum of 10 locations+ var locs: [10]Location = undefined;+ var loc_it = std.mem.splitSequence(u8, raw_arg, ",");+ while (loc_it.next()) |loc_str| : (loc_cnt += 1) {+ if (loc_cnt >= 10) {+ log.err("too many locations in cycle, max 10", .{});+ return;+ }+ locs[loc_cnt] = std.meta.stringToEnum(Location, loc_str) orelse {+ log.err("unknown location: {s}", .{loc_str});+ return;+ };+ }+ var fi: i8 = -1;+ for (0.., locs[0..loc_cnt]) |i, loc| {+ if (output.cfg.main_location == loc) {+ fi = @intCast(i);+ break;+ }+ }+ fi = fi + 1;+ if (fi == loc_cnt) {+ fi = 0;+ }+ output.cfg.main_location = locs[@intCast(fi)];+ }, .@"main-count" => {
const arg = fmt.parseInt(i32, raw_arg, 10) catch |err| {
log.err("failed to parse argument: {}", .{err});
--
2.44.0
I'm not against adding this feature but I don't get the point of adding
a list of 10 locations. I would prefer to just allow cycling through the
5 locations, since you can't add custom locations, unless you have a
good reason for adding it like this?
On 3/11/24 09:44, Hugo Machet wrote:
> I'm not against adding this feature but I don't get the point of adding> a list of 10 locations. I would prefer to just allow cycling through the> 5 locations, since you can't add custom locations, unless you have a> good reason for adding it like this?
I personally only use left and monocle, so cycling the others is a waste. This
lets you follow more closely XMonad (where you can cycle the layouts you use),
I've used it for more than 10y so kinda hard to change. Also the mirrored ones
are pretty confusing (because the increase/decrease commands work in reverse
etc).
One thing I can definitely improve is drop the array and only use the iterator,
the code will be shorter/more concise.
> I personally only use left and monocle, so cycling the others is a waste. This> > lets you follow more closely XMonad (where you can cycle the layouts you use),> > I've used it for more than 10y so kinda hard to change. Also the mirrored ones> > are pretty confusing (because the increase/decrease commands work in reverse> > etc).
That's fair, though I think we should only allow a maximum of 5
locations then, I don't think there is a use case to have the same one
multiple time?
One thing we could do instead is using a flag -main-location cycle <list>,
and add a default list in Config of:
`cycle-list: []const u8: "top,right,bottom,left,monocle";`
and then the command `main-location-cycle` would just cycle through the
list without taking an argument. It would not be configurable at
runtime in this case though, not sure if it is really a problem, we
could also just still take a list argument in the command to change the
list at runtime of course, maybe a optional args.
What do you think of this as someone who use that?
After we decided what to use, the man page should be updated too
although since this is an annoying format I'll do it after if you don't
want, no worries.
> > One thing I can definitely improve is drop the array and only use the iterator,> > the code will be shorter/more concise.>
I haven't really thought of the code yet so if you have an idea for a
more concise way, feel free to do so.
I also updated the .build.yml that was messed up if you want to rebase
On 3/11/24 11:23, Hugo Machet wrote:
>> I personally only use left and monocle, so cycling the others is a waste. This>>>> lets you follow more closely XMonad (where you can cycle the layouts you use),>>>> I've used it for more than 10y so kinda hard to change. Also the mirrored ones>>>> are pretty confusing (because the increase/decrease commands work in reverse>>>> etc).>>> That's fair, though I think we should only allow a maximum of 5> locations then, I don't think there is a use case to have the same one> multiple time?
I'm new to zig (this was my first try), I wanted to use a vector (dynamic
array), couldn't figure it out, then stuck 10 just to try it out with a static
array. As I said, if implemented better (i.e without array), it won't have this
weird limitation. I agree it's weird that you support 5 in total, and I put
arbitrary 10.
> One thing we could do instead is using a flag -main-location cycle <list>,> and add a default list in Config of:>> `cycle-list: []const u8: "top,right,bottom,left,monocle";`>> and then the command `main-location-cycle` would just cycle through the> list without taking an argument. It would not be configurable at> runtime in this case though, not sure if it is really a problem, we> could also just still take a list argument in the command to change the> list at runtime of course, maybe a optional args.> What do you think of this as someone who use that?
I think the command "pick the next in the list or the first one, if current is
not in the list" is a well defined useful operation. We could make a single
at-start defined cycle, but that is less general, and in terms of
implementation - not simpler. Alternatively we can just add support for the
cycling in the existing command. It is backwards compatible (if there is no
comma, it behaves exactly as picking a single layout).
However, you're the boss, so I'd agree with whatever you decide (i.e define
a loop at start and have a special command to pick from the pre-defined loop).
Using it inside river init, you'd have to put the loop once, either in the args
to rivercarro or to the map command, and the map variant (my suggestion) is
a bit more versatile (i.e you can have multiple cycles, say one for bigger
screens one for smaller).
> After we decided what to use, the man page should be updated too> although since this is an annoying format I'll do it after if you don't> want, no worries.
I can look into that as well, just wanted to start the discussion first :)
>>>> One thing I can definitely improve is drop the array and only use the iterator,>>>> the code will be shorter/more concise.>>>> I haven't really thought of the code yet so if you have an idea for a> more concise way, feel free to do so.
I'll definitely freshen up the code, now I know how loops work at least :-D
> I also updated the .build.yml that was messed up if you want to rebase
Awesome, thanks!
Alright, we stay on your first idea for now, a command
`main-location-cycle` that takes a comma separated list of max 5
locations, if there stll a need for this limitation.
For the implementations I let you decide what you want, note that you
don't need to make something too complex, shorter and more concise code
is nice if it doesn't add too much complexity. Static array or
something like std.ArrayListUnmanaged() are not bad :)