Skip to content

Conversation

@liskin
Copy link

@liskin liskin commented May 25, 2021

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.

liskin added 2 commits May 26, 2021 00:23
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.
This one makes it easier to see which lines are separate messages and
which are just a multiline message.

Inspired (actually shamelessly copy-pasted) by
wee-slack/wee-slack@adb1029
that fixes the same issue in wee-slack.

Example before:

    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.

Example after the fix:

    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.

This unfortunately breaks prefix_same_nick for multiline messages again.
(Probably not a big deal. Certainly not for me.)
@poljar
Copy link
Owner

poljar commented Jun 2, 2021

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
Copy link
Author

liskin commented Jun 2, 2021

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: wee-slack/wee-slack#357 (comment)). Feel free to cherry-pick either patch into master, I'll carry the rest in my fork.

@trygveaa
Copy link
Contributor

trygveaa commented Jun 6, 2021

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
Copy link
Contributor

trygveaa commented Jun 6, 2021

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
Copy link
Author

liskin commented Jun 6, 2021

@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: liskin@42f1b86, because I really hated the <        > prefixes in multiline messages. That one definitely breaks prefix_same_nick. Possibly my confusion came from there.)

@trygveaa
Copy link
Contributor

trygveaa commented Jun 6, 2021

(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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants