Skip to content
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

buffer_autoset.py's functionality should be in WeeChat itself #352

Closed
0xallie opened this issue Mar 8, 2015 · 30 comments
Closed

buffer_autoset.py's functionality should be in WeeChat itself #352

0xallie opened this issue Mar 8, 2015 · 30 comments
Assignees
Labels
feature New feature request
Milestone

Comments

@0xallie
Copy link
Contributor

0xallie commented Mar 8, 2015

Currently, buffer_autoset.py is the only way to save buffer settings.

@flashcode flashcode added the feature New feature request label Mar 8, 2015
@highflyer77
Copy link

I agree 100%, I was going to leave the same comment.
Also would like to note:
Some scripts simply should be the default functionality and this is one of them.
histman.py would be another, and requires buffer_autoset.py to work.

@lucy
Copy link

lucy commented Oct 21, 2019

Could this be part of the trigger plugin?

@weechatter
Copy link
Contributor

it should be part of core.

@flashcode flashcode self-assigned this Aug 18, 2023
@flashcode flashcode added this to the 4.1.0 milestone Aug 18, 2023
@flashcode flashcode added the in progress Someone is working on this issue label Aug 18, 2023
@flashcode
Copy link
Member

Hi,

This is scheduled for 4.1.0.

Question to the users of the script, which properties are you setting in your buffers?

I ask because the script uses a timer to delay the set of variables.
I think this was needed, especially on IRC channels, before introduction of function buffer_new_props in API (used to open a buffer and set properties in an atomic way, before the signal "buffer_opened" is sent).

So if possible, I'll reimplement the feature without using a timer, so the workflow will be like this:

  1. call buffer_new_props or buffer_new (with or without properties to set)
  2. function creates the buffer and set properties (if given)
  3. function sets extra properties: options weechat.buffer.* (purpose of this issue)
  4. function sends signal "buffer_opened"
  5. in some cases, plugins may set extra properties after (if that's the case, it could overwrite properties added by the user in option weechat.buffer.xxx and then it could be a problem).

New options are weechat.buffer.<name>.<property> where name is a buffer name or mask (* allowed) and property a buffer property (any property that can be set with function buffer_set.

The value of option is evaluated (it was not done by the script).

The command /buffer set <property> <value> will automatically create the option weechat.buffer.<name>.<property> (<name> being the buffer name where the command is executed) to the value <value>.
This could be made optional with a new option (is it needed?): the boolean option could be enabled by default, and if disabled, the /buffer set command doesn't create any option.

Examples:

# add highlight on "weechat" on all IRC channels
/set weechat.buffer.irc.*.highlight_words_add "${if:${type}==channel?weechat}"

# disable highlight from "weebot" on libera channels
/set weechat.buffer.irc.libera.*.hotlist_max_level_nicks_add "weebot:-1"

# set short name on channel #very_long_channel_name
/set weechat.buffer.irc.libera.#very_long_channel_name.short_name "#longchan"

@pascalpoitras
Copy link

mine are: notify, highlight_words, title, time_for_each_line, short_name, clear, key_bind, localvar_set, hidden

@0xallie
Copy link
Contributor Author

0xallie commented Aug 18, 2023

hotlist_max_level_nicks_add, highlight_words, notify and `localvar_set‘ come to mind for me.

@flashcode
Copy link
Member

Thanks @pascalpoitras & @0xallie, which local variables are you setting? (just to be sure they can't get overwritten after setting them with these new options)

@pascalpoitras
Copy link

@flashcode only custom one not those provided by weechat except if short_name is considered a var. I mean it is display in /buffer listvar but still it is not set using localvar_set.

@flashcode
Copy link
Member

@pascalpoitras: OK.
No, short_name is not a local variable and should not be displayed by /buffer listvar (if it is, that means a new local variable with this name has been added somehow).

@pascalpoitras
Copy link

@flashcode ah my bad it's name not short_name

@flashcode
Copy link
Member

Yes plugin and name are added as local variables (even if name is also a buffer property by itself).

@trygveaa
Copy link
Contributor

New options are weechat.buffer.. where name is a buffer name or mask (* allowed)

Will the options for a buffer be renamed if the buffer is renamed?

and property a buffer property (any property that can be set with function buffer_set.

Should we distinguish between buffer properties and localvars, or should one just use localvar_set_ in the option?

@weechatter
Copy link
Contributor

I am using this:
buffer_autoset.buffer.irc.libera.#weechat.time_for_each_line
buffer_autoset.buffer.irc.libera.#weechat-de.highlight_words_add
buffer_autoset.buffer.irc.libera.hotlist_max_level_nicks_add

@flashcode
Copy link
Member

Will the options for a buffer be renamed if the buffer is renamed?

Not planned, and not easy as you can use mask as well (this is not done by the script buffer_autoset.py).

Should we distinguish between buffer properties and localvars, or should one just use localvar_set_ in the option?

localvar_set_xxx must be used.

@pascalpoitras
Copy link

pascalpoitras commented Aug 18, 2023

@flashcode

You're asking if we should add an option to control the behavior of /buffer set (permanent or not)

so we should maybe do this instead

  1. keep /buffer set the way it currently work
  2. add a command /buffer_autoset (yes the same name as the script)

then when we want it to be permanent we use the /buffer_autoset command?

@flashcode
Copy link
Member

  1. add a command /buffer_autoset (yes the same name as the script)

then when we want it to be permanent we use the /buffer_autoset command?

@pascalpoitras: then it could just be a parameter like /buffer setauto <property> <value> instead of a new command, no?

@pascalpoitras
Copy link

@flashcode yes, one or another is ok with me

@pascalpoitras
Copy link

pascalpoitras commented Aug 18, 2023

@flashcode

but /buffer doesn't take name juste a property and value so how to match multiple buffer ?

EDIT:

I guess we could use /command -buffer

@flashcode
Copy link
Member

Or syntax could be different: /buffer setauto <buffer> <property> <value>

@flashcode
Copy link
Member

flashcode commented Aug 19, 2023

I pushed a branch https://github.com/weechat/weechat/tree/options-buffer-properties for tests.

For now it just adds the new options weechat.buffer.* that can be added manually with /set, there's no other command to create them yet.

The properties are applied only when the buffers are opened.

@pascalpoitras
Copy link

tested, works great

@flashcode
Copy link
Member

To stay consistent with other /buffer parameters, I propose that the new parameter, like the others, applies on the current buffer only.
So if you want to use a mask to target multiple buffers, you have to use this command (that will be mentioned in /help buffer):

/set weechat.buffer.<mask>.<property> "<value>"

That means the following parameter is going to be added:

/buffer setauto <property> <value>

The property will be completed with all properties (like /buffer set) with in addition the local variables, using the format: localvar_set_xxx (xxx being an existing buffer local variable).

The property is immediately applied in the current buffer, and option weechat.buffer.<name>.<property> is set the given value (option is created or updated if already existing).

The option must be removed with /unset weechat.buffer.<name>.<property> and nothing happens in the buffer when the option is removed.

@weechatter
Copy link
Contributor

Sounds good!

@flashcode
Copy link
Member

Done, branch merged to master.

Your feedback is welcome, please test with your existing buffer_autoset config and report any issue, thanks!

@flashcode flashcode removed the in progress Someone is working on this issue label Aug 24, 2023
@trygveaa
Copy link
Contributor

Great!

I noticed that if you don't provide a value, the option is not considered changed by fset (not colored, doesn't show up when listing diff with d). I think it should be.

I tried using this for changing the key bindings in fset, and it changes the binding when you set the option or use setauto, but if i close and reopen the buffer, the default bindings are back. So maybe the options are applied before the default fset bindings are set?

I would also like a way for users to discover what key bindings are set in a buffer, and maybe a more user friendly way of changing them (or at least a documented way). For normal key bindings, all options for them are created and shown in fset, so that's an option. However, this might not be a good approach for these dynamic options, as a lot of them might be created if you have many buffers with custom bindings (e.g. if the irc plugin or wee-slack created key bindings for their normal buffers), which would create a lot of clutter. So not sure what the best solution is here.

The property will be completed with all properties (like /buffer set) with in addition the local variables, using the format: localvar_set_xxx (xxx being an existing buffer local variable).

Why are localvars only completed for setauto, and not set?

Also, would be nice if key_bind_xxx and key_unbind_xxx for the current bindings autocompleted with both as well.

@flashcode
Copy link
Member

I noticed that if you don't provide a value, the option is not considered changed by fset (not colored, doesn't show up when listing diff with d). I think it should be.

I'll check how to do that, but not so easy: the default value of these options is empty string, and if you create an option with empty string, it's then considered as not changed on fset buffer.
Any idea how to do this?

I tried using this for changing the key bindings in fset, and it changes the binding when you set the option or use setauto, but if i close and reopen the buffer, the default bindings are back. So maybe the options are applied before the default fset bindings are set?

Yes, I'll change this in fset plugin to define keys during the buffer creation, so user options will always apply after.
I'll also fix other buffers if needed (/script and /list at least, maybe some others).

I would also like a way for users to discover what key bindings are set in a buffer, and maybe a more user friendly way of changing them (or at least a documented way). For normal key bindings, all options for them are created and shown in fset, so that's an option. However, this might not be a good approach for these dynamic options, as a lot of them might be created if you have many buffers with custom bindings (e.g. if the irc plugin or wee-slack created key bindings for their normal buffers), which would create a lot of clutter. So not sure what the best solution is here.

Yes to be discussed, not sure using these options is the best approach for this.

The property will be completed with all properties (like /buffer set) with in addition the local variables, using the format: localvar_set_xxx (xxx being an existing buffer local variable).

Why are localvars only completed for setauto, and not set?

Because local variables are manager with /buffer setvar and /buffer delvar, while /buffer setauto can set properties and local variables.
Is it an issue?

Also, would be nice if key_bind_xxx and key_unbind_xxx for the current bindings autocompleted with both as well.

Yes, will add this.

@trygveaa
Copy link
Contributor

I'll check how to do that, but not so easy: the default value of these options is empty string, and if you create an option with empty string, it's then considered as not changed on fset buffer. Any idea how to do this?

Hm, okay, I haven't looked into it, so not sure. What's the reason new options with an empty value is not considered changed?

Because local variables are manager with /buffer setvar and /buffer delvar, while /buffer setauto can set properties and local variables. Is it an issue?

Ah, right, makes sense.

Thanks for the other fixes.

@flashcode
Copy link
Member

I'll check how to do that, but not so easy: the default value of these options is empty string, and if you create an option with empty string, it's then considered as not changed on fset buffer. Any idea how to do this?

Hm, okay, I haven't looked into it, so not sure. What's the reason new options with an empty value is not considered changed?

There's no easy way in API to know it's a "new" option, vs an option that exists by default.
So fset just shows changed options if the current value is different from the default value.
There's the same issue for options plugins.var.xxx by the way.
I'll check how this could be fixed.

@lacygoill
Copy link

Maybe it's working as intended, but in order to bind ctrl-n to select the next option in the /fset buffer or jump to the next buffer in the buflist depending on whether the latter is hidden, I need to write /eval and to escape the ${variables}:

/set weechat.buffer.fset.fset.key_bind_ctrl-n "/eval \${if:\${weechat.bar.buflist.hidden} ?/fset -down :/buffer +1}"
                                               ^---^ ^     ^

Without, ctrl-n unconditionally executes /fset -down or /buffer +1. As if the option's value was evaluated immediately when it was being set. It seems weird because:

  • I have 16 other options whose values contain ${variables}, and none of them requires to delay the evaluation
  • I didn't have to delay the evaluation when was I using the buffer_autoset.py plugin to bind ctrl-n with the same purpose
  • if I ask for the value of weechat.buffer.fset.fset.key_bind_ctrl-n, without using /eval and the backslashes, the displayed value is "${if:${weechat.bar.buflist.hidden} ?/fset -down :/buffer +1}"; ${weechat.bar.buflist.hidden} hasn't already been evaluated and the ${if:} condition is still there

@flashcode
Copy link
Member

flashcode commented Aug 30, 2023

@lacygoill: good catch, this is because now all buffer properties (including "key_bind_xxx") are immediately evaluated, while they were not with buffer_autoset.py.

There are multiple possible solutions, I think first one is the best:

  1. Do not evaluate properties key_bind_xxx or key_unbind_xxx (even if this one doesn't require any value), so this lets you use /eval without escaping anything.
  2. Try to auto-detect if eval is required by checking if ${ is present and not escaped, evaluate value only if present.
  3. Add a way to explicitly enable or disable eval on a property (to define how).
  4. Never evaluate options weechat.buffer.* (like buffer_autoset.py).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature request
Projects
None yet
Development

No branches or pull requests

7 participants