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

change the statusbar commands so that no accidential status bars are created #858

Merged
merged 7 commits into from Apr 2, 2018

Conversation

Projects
None yet
2 participants
@ailin-nemui
Copy link
Contributor

ailin-nemui commented Mar 11, 2018

New command suggestion:

Syntax

STATUSBAR ADD|MODIFY [-disable | -nodisable] [-type window|root] [-placement top|bottom] [-position #] [-visible always|active|inactive] <statusbar>

STATUSBAR RESET <statusbar>

STATUSBAR ADDITEM|MODIFYITEM [-before | -after <item>] [-priority #] [-alignment left|right] <item> <statusbar>

STATUSBAR REMOVEITEM <item> <statusbar>

STATUSBAR INFO <statusbar>

Parameters

ADD: Adds a statusbar to the list of statusbars.
MODIFY: Modifies the configuration of a statusbar.
RESET: Restores the default statusbar configuration.
ADDITEM: Adds an item to the specified statusbar. It can be set to appear before/after another item and left/right aligned to a specified position on the screen.
MODIFYITEM: Changes an item position inside a bar.
REMOVEITEM: Removes an item from the specified statusbar.
INFO: List the current details and items of the specified statusbar.
-disable: Removes a statusbar from the list.
-type: Sets the type of statusbar, for each split window or only for the current root screen.
-placement: Sets the placement of the statusbar, either at the top or the bottom of the screen.
-position: Sets the position of the statusbar. Represented as a number, with 0 implying the first position.
-visible: Sets the visibility of the statusbar or item. If set to always it is visible on all screens, otherwise if set to inactive or active then it is only visible on inactive or active screens, respectively.
-before: This item is added before the other item.
-after: This item is added after the other item.
-priority: When the statusbar items overflow, the item with the lowest priority is removed first
-alignment: Display the item on the right side.

Where statusbar refers to the name of the statusbar; if no argument is
given, the entire list of statusbars will be displayed.

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 11, 2018

@dequis not sure about additem subcommand, thoughts?

@dequis

This comment has been minimized.

Copy link
Member

dequis commented Mar 11, 2018

Before reading too much: hell yeah we needed this, the statusbar commands suck

@ailin-nemui ailin-nemui changed the title change the statusbar commands so that no accidenal status bars are created change the statusbar commands so that no accidential status bars are created Mar 12, 2018

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 15, 2018

@irssi/developers if there isn't any further comment I suggest this for merging

@dequis
Copy link
Member

dequis left a comment

The examples section of the /help is outdated. Also worth mentioning there that the commands changed and the old syntax is still valid but not documented.

cmd_params_free(free_arg);
return;
}

node = statusbar_items_section(node);
if (node == NULL)
return;

This comment has been minimized.

@dequis

dequis Mar 19, 2018

Member

Missing cmd_params_free(free_arg);

This comment has been minimized.

@ailin-nemui

ailin-nemui Mar 19, 2018

Contributor

good catch

static void cmd_statusbar_add(const char *data, void *server,
void *item, CONFIG_NODE *node)
/* SYNTAX: STATUSBAR ADDITEM|MODIFYITEM [-before | -after <item>]
[-priority #] [-alignment left|right] <item> <statusbar> */

This comment has been minimized.

@dequis

dequis Mar 19, 2018

Member

Not sure about additem being <item> first <statusbar> last

This comment has been minimized.

@ailin-nemui

ailin-nemui Mar 19, 2018

Contributor

it somewhat matches /channel add <channel> <network>

This comment has been minimized.

@dequis

dequis Mar 19, 2018

Member

I guess. No strong opinion either way.

void *item, CONFIG_NODE *node)
{
iconfig_node_set_int(node, "position", atoi(data));
node = sbar_node(name, TRUE);

This comment has been minimized.

@dequis

dequis Mar 19, 2018

Member

If i'm reading this right both add and modify call sbar_node with create=TRUE. Same thing with additem/modifyitem.

This comment has been minimized.

@ailin-nemui

ailin-nemui Mar 19, 2018

Contributor

oops

command_bind("statusbar info", NULL, (SIGNAL_FUNC) cmd_statusbar_info);
command_bind("statusbar additem", NULL, (SIGNAL_FUNC) cmd_statusbar_additem_modifyitem);
command_bind("statusbar modifyitem", NULL, (SIGNAL_FUNC) cmd_statusbar_additem_modifyitem);
command_bind("statusbar removeitem", NULL, (SIGNAL_FUNC) cmd_statusbar_removeitem);

This comment has been minimized.

@dequis

dequis Mar 19, 2018

Member

space indents

command_unbind("statusbar info", (SIGNAL_FUNC) cmd_statusbar_info);
command_unbind("statusbar additem", (SIGNAL_FUNC) cmd_statusbar_additem_modifyitem);
command_unbind("statusbar modifyitem", (SIGNAL_FUNC) cmd_statusbar_additem_modifyitem);
command_unbind("statusbar removeitem", (SIGNAL_FUNC) cmd_statusbar_removeitem);

This comment has been minimized.

@dequis

dequis Mar 19, 2018

Member

space indents

node = statusbar_items_section(node);
if (node == NULL) {
cmd_params_free(free_arg);
return;

This comment has been minimized.

@dequis

dequis Mar 19, 2018

Member

space indents

parent = iconfig_node_section(parent, active_statusbar_group->name,
NODE_TYPE_BLOCK);

iconfig_node_set_str(parent, node->key, NULL);

This comment has been minimized.

@dequis

dequis Mar 19, 2018

Member

space indents

return NULL;
}

/* lookup/create the statusbar node */

This comment has been minimized.

@dequis

dequis Mar 19, 2018

Member

space indents

This comment has been minimized.

@ailin-nemui

ailin-nemui Mar 19, 2018

Contributor

.. I thought I had ran the almight git-clang-format ..

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 19, 2018

thanks for review

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 19, 2018

any suggestion as to where/which position to mention the change in syntax?

@dequis

This comment has been minimized.

Copy link
Member

dequis commented Mar 19, 2018

I was thinking a single-sentence mention in the Description section of the /help, without going in detail

@ailin-nemui ailin-nemui force-pushed the ailin-nemui:sbar branch from 9390d8b to ee04cd5 Mar 20, 2018

@ailin-nemui ailin-nemui force-pushed the ailin-nemui:sbar branch from 6f9850f to 138eca0 Mar 20, 2018

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 20, 2018

@dequis wow that was more complicated than I thought. it could need another look. Basically the old methods were hard-wired to always create and copy the status bar items, so I had to change that with tests for the internal/default config and only copying if we actually are about to change something

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Mar 26, 2018

@ailin-nemui ailin-nemui merged commit 8bf2162 into irssi:master Apr 2, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@ailin-nemui ailin-nemui deleted the ailin-nemui:sbar branch Sep 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment