search

poljar / weechat-matrix

Weechat Matrix protocol script written in python
Other
957 stars 119 forks source link

buffer: Fix prefix of multiline messages #273

Open liskin opened 3 years ago

liskin commented 3 years ago

This primarily fixes alignment of multiline messages when weechat.look.prefix_align is set to "none". Messages without prefix would otherwise be aligned with the nickname instead of being aligned with the message.

Example before:

20:20:00 <abcdefg> > In reply to @xyz:matrix.org
20:20:00 > Lorem ipsum dolor sit amet, consectetur adipiscing elit.
20:20:00 Sed accumsan nisi vel vestibulum hendrerit.
20:21:00 <xyz> Cras a lacus libero.

Example after the fix:

20:20:00 <abcdefg> > In reply to @xyz:matrix.org
20:20:00 <abcdefg> > Lorem ipsum dolor sit amet, consectetur adipiscing elit.
20:20:00 <abcdefg> Sed accumsan nisi vel vestibulum hendrerit.
20:21:00 <xyz> Cras a lacus libero.

With weechat.look.prefix_align being set to the default "right", the additional lines would be printed prefixed with "<>" and weechat.look.{prefix_same_nick,prefix_same_nick_middle} weren't taken into account. Now they are.

poljar commented 3 years ago

I don't think we should break prefix_same_nick, we could merge the first commit though, I think the Rust plugin does the same thing as the first commit.

liskin commented 3 years ago

I can understand that. Breaking prefix_same_nick is unfortunately necessary if one wants to tell individual multiline messages apart (see my discussion with the wee-slack maintainer here: https://github.com/wee-slack/wee-slack/issues/357#issuecomment-848381277). Feel free to cherry-pick either patch into master, I'll carry the rest in my fork.

trygveaa commented 3 years ago

What do you mean that prefix_same_nick is broken? It seems to work fine for me with this change.

Breaking prefix_same_nick is unfortunately necessary if one wants to tell individual multiline messages apart.

When prefix_same_nick is set, you can't tell different multiline messages apart in weechat-matrix, since it doesn't do the same as what wee-slack does to ensure that. This doesn't change with this PR, it's a different things wee-slack does than what you copied that handles that. And I don't think prefix_same_nick is broken in wee-slack either (unless you consider a blank prefix for subsequent lines of a multiline message to be breaking prefix_same_nick).

As far as I know, this change shouldn't affect prefix_same_nick at all, because when that is set and prefix_nick_... tags are used, the prefix for subsequent lines with the same prefix_nick_... tag is completely ignored, and the value of prefix_same_nick is used instead.

trygveaa commented 3 years ago

This also means that the alignment won't be fixed by this PR when prefix_same_nick is set, only when it's not. Only way to fix that would be to do the same as what wee-slack does (though, that has some other issues). But I don't think that should be a reason not to merge this, since it's better to at least fix it when prefix_same_nick is not set.

liskin commented 3 years ago

@trygveaa Oh, you're right, weechat uses the prefix_nick_ tags to tell if two messages are from the same nick, not the actual line prefixes. I should change the commit message then.

(I actually run one extra commit on top of this PR, which does something a bit closer to what wee-slack does: https://github.com/liskin/weechat-matrix/commit/42f1b8631fb943eb0de49238c9986d39b3b02741, because I really hated the <        > prefixes in multiline messages. That one definitely breaks prefix_same_nick. Possibly my confusion came from there.)

trygveaa commented 3 years ago

(I actually run one extra commit on top of this PR, which does something a bit closer to what wee-slack does: liskin@42f1b86, because I really hated the <        > prefixes in multiline messages. That one definitely breaks prefix_same_nick. Possibly my confusion came from there.)

Oh, I see that happens when weechat.look.nick_prefix/suffix is set, and in bare mode. That doesn't look very good, so I suppose that might be a reason to not include the second commit, unless you want to go the wee-slack route that avoids this issue.