Use list_find in more places and refactor/fix workspace prev_next functions

The original purpose of this commit is to replace some for loops with
list_find. But while doing this I found the workspace_prev_next_impl
functions to be difficult to read and also contained a bug, so I
refactored them and fixed the bug.

To reproduce the bug:

* Have two outputs, where the left output has workspaces 1, 2, 3 and the
right output has workspaces 4, 5, 6. Make workspace 2 focused_inactive
and workspace 4 focused.
* Run `workspace prev`.
* Previously it would visit the left output, then apply `workspace prev`
to workspace 2, which focuses workspace 1.
* Now it will focus the rightmost workspace on the left output
(workspace 3).

The refactoring I made to the workspace functions are:

* Added the static keyword.
* They now accept an int dir rather than bool, to avoid an unnecessary
conversion.
* Rather than preparing start and end variables for the purpose of
iterating, just iterate everything.
* Replace for loops with list_find.
* Don't call workspace_output_prev_next_impl (this fixes the bug).
This commit is contained in:
Ryan Dwyer 2018-08-15 15:14:35 +10:00
parent b4887ba154
commit 701fcafc70
5 changed files with 37 additions and 70 deletions

View file

@ -77,7 +77,7 @@ int list_seq_find(list_t *list, int compare(const void *item, const void *data),
return -1;
}
int list_find(list_t *list, void *item) {
int list_find(list_t *list, const void *item) {
for (int i = 0; i < list->length; i++) {
if (list->items[i] == item) {
return i;

View file

@ -20,7 +20,7 @@ void list_qsort(list_t *list, int compare(const void *left, const void *right));
// Return index for first item in list that returns 0 for given compare
// function or -1 if none matches.
int list_seq_find(list_t *list, int compare(const void *item, const void *cmp_to), const void *cmp_to);
int list_find(list_t *list, void *item);
int list_find(list_t *list, const void *item);
// stable sort since qsort is not guaranteed to be stable
void list_stable_sort(list_t *list, int compare(const void *a, const void *b));
// swap two elements in a list

View file

@ -20,14 +20,7 @@
#include "log.h"
static int index_child(const struct sway_container *child) {
struct sway_container *parent = child->parent;
for (int i = 0; i < parent->children->length; ++i) {
if (parent->children->items[i] == child) {
return i;
}
}
// This happens if the child is a floating container
return -1;
return list_find(child->parent->children, child);
}
static void container_handle_fullscreen_reparent(struct sway_container *con,
@ -125,11 +118,9 @@ struct sway_container *container_remove_child(struct sway_container *child) {
}
struct sway_container *parent = child->parent;
for (int i = 0; i < parent->children->length; ++i) {
if (parent->children->items[i] == child) {
list_del(parent->children, i);
break;
}
int index = index_child(child);
if (index != -1) {
list_del(parent->children, index);
}
child->parent = NULL;
container_notify_subtree_changed(parent);

View file

@ -84,11 +84,9 @@ void root_scratchpad_remove_container(struct sway_container *con) {
return;
}
con->scratchpad = false;
for (int i = 0; i < root_container.sway_root->scratchpad->length; ++i) {
if (root_container.sway_root->scratchpad->items[i] == con) {
list_del(root_container.sway_root->scratchpad, i);
break;
}
int index = list_find(root_container.sway_root->scratchpad, con);
if (index != -1) {
list_del(root_container.sway_root->scratchpad, index);
}
}

View file

@ -286,8 +286,8 @@ struct sway_container *workspace_by_name(const char *name) {
* the end and beginning. If next is false, the previous workspace is returned,
* otherwise the next one is returned.
*/
struct sway_container *workspace_output_prev_next_impl(
struct sway_container *output, bool next) {
static struct sway_container *workspace_output_prev_next_impl(
struct sway_container *output, int dir) {
if (!output) {
return NULL;
}
@ -302,27 +302,17 @@ struct sway_container *workspace_output_prev_next_impl(
focus :
container_parent(focus, C_WORKSPACE));
int i;
for (i = 0; i < output->children->length; i++) {
if (output->children->items[i] == workspace) {
return output->children->items[
wrap(i + (next ? 1 : -1), output->children->length)];
}
}
// Doesn't happen, at worst the for loop returns the previously active
// workspace
return NULL;
int index = list_find(output->children, workspace);
size_t new_index = wrap(index + dir, output->children->length);
return output->children->items[new_index];
}
/**
* Get the previous or next workspace. If the first/last workspace on an output
* is active, proceed to the previous/next output's previous/next workspace. If
* next is false, the previous workspace is returned, otherwise the next one is
* returned.
* is active, proceed to the previous/next output's previous/next workspace.
*/
struct sway_container *workspace_prev_next_impl(
struct sway_container *workspace, bool next) {
static struct sway_container *workspace_prev_next_impl(
struct sway_container *workspace, int dir) {
if (!workspace) {
return NULL;
}
@ -331,52 +321,40 @@ struct sway_container *workspace_prev_next_impl(
return NULL;
}
struct sway_container *current_output = workspace->parent;
int offset = next ? 1 : -1;
int start = next ? 0 : 1;
int end;
if (next) {
end = current_output->children->length - 1;
struct sway_container *output = workspace->parent;
int index = list_find(output->children, workspace);
int new_index = index + dir;
if (new_index >= 0 && new_index < output->children->length) {
return output->children->items[index + dir];
}
// Look on a different output
int output_index = list_find(root_container.children, output);
new_index = wrap(output_index + dir, root_container.children->length);
output = root_container.children->items[new_index];
if (dir == 1) {
return output->children->items[0];
} else {
end = current_output->children->length;
return output->children->items[output->children->length - 1];
}
int i;
for (i = start; i < end; i++) {
if (current_output->children->items[i] == workspace) {
return current_output->children->items[i + offset];
}
}
// Given workspace is the first/last on the output, jump to the
// previous/next output
int num_outputs = root_container.children->length;
for (i = 0; i < num_outputs; i++) {
if (root_container.children->items[i] == current_output) {
struct sway_container *next_output = root_container.children->items[
wrap(i + offset, num_outputs)];
return workspace_output_prev_next_impl(next_output, next);
}
}
// Doesn't happen, at worst the for loop returns the previously active
// workspace on the active output
return NULL;
}
struct sway_container *workspace_output_next(struct sway_container *current) {
return workspace_output_prev_next_impl(current, true);
return workspace_output_prev_next_impl(current, 1);
}
struct sway_container *workspace_next(struct sway_container *current) {
return workspace_prev_next_impl(current, true);
return workspace_prev_next_impl(current, 1);
}
struct sway_container *workspace_output_prev(struct sway_container *current) {
return workspace_output_prev_next_impl(current, false);
return workspace_output_prev_next_impl(current, -1);
}
struct sway_container *workspace_prev(struct sway_container *current) {
return workspace_prev_next_impl(current, false);
return workspace_prev_next_impl(current, -1);
}
bool workspace_switch(struct sway_container *workspace,