Make topic headers case-insensitive

[shivanshsharma13]

Topics in Zulip are case-insensitive. This change makes the message list's topic headers match that behavior, so messages whose topics differ only in case (like "missing string" and "Missing string") share a single header. This brings the behavior in line with Zulip web.

What is the change?

The change modifies the topic comparison logic in haveSameRecipient() to use case-insensitive comparison when determining whether to show a new recipient header.

Screenshots

Before	After

Fixes #739

[chrisbobbe]

Hi, thanks for the PR! Please revise the commit message to follow our style (we have a useful tip that will help you see examples of mergeable commit messages in the project's history). Also, please write a test for this bugfix; that should go in the same commit that adds the code being tested.

[shivanshsharma13]

Implemented the requested changes. Please let me know if there’s anything else that needs adjustment. Thanks

[chrisbobbe]

Thanks!

For the commit message:

Message_list: Make topic headers case-insensitive

This change in the commit makes the message list's topic headers match that behavior, so messages whose topics differ only in case (like "missing string" and "Missing string") share a single header.  This brings the behavior in line with Zulip web. (Added the relevant test case)

We use the prefix msglist: for commits about the message list; see examples using the tip I shared earlier for browsing history.

Also let's make the summary line more explicit about the bug it fixes. How about:

msglist: Remove extra recipient header when topic changes case

Also your commit is missing a Fixes: line and isn't word-wrapped properly.

[chrisbobbe]

nit: put this new test just under the test 'stream messages match just if same stream/topic'

[chrisbobbe]

Also here's a version that has different tests for each case (instead of multiple checks in the same test), along with a descriptive label for each test, which will appear in output when it fails. This also tests some additional cases that are interesting:

    group('topics compared case-insensitively', () {
      void doTest(String description, String topicA, String topicB, bool expected) {
        test(description, () {
          final stream = eg.stream();
          final messageA = eg.streamMessage(stream: stream, topic: topicA);
          final messageB = eg.streamMessage(stream: stream, topic: topicB);
          check(haveSameRecipient(messageA, messageB)).equals(expected);
        });
      }

      doTest('same case, all lower',               'abc',  'abc',  true);
      doTest('same case, all upper',               'ABC',  'ABC',  true);
      doTest('same case, mixed',                   'AbC',  'AbC',  true);
      doTest('same non-letter chars',              '嗎',    '嗎',    true);
      doTest('different case',                     'aBc',  'ABC',  true);
      doTest('different case, same diacritics',    'AbÇ',  'aBç',  true);
      doTest('same letters, different diacritics', 'ma',   'mǎ',   false);
      doTest('having different non-letter chars',  'ma 嗎', 'mǎ 馬', false);
    });

[shivanshsharma13]

Yes, thank you for the help! This test looks much more promising and thoroughly checks each case. I'll replace my current implementation with this one and ensure to take care of such details from next time onward.

[chrisbobbe]

Thanks! Please wrap the commit-message body to 68 characters and format the "Fixes" line the same way we do in other commits (see the project's history for examples).

[shivanshsharma13]

Thanks! Please wrap the commit-message body to 68 characters and format the "Fixes" line the same way we do in other commits (see the project's history for examples).

Done, Please review.

• Wrapped the cotent in 68 chars a line
• Used Fixes: format as done on other commits (sorry for overlooking previously)

[chrisbobbe]

Thanks! Marking for Greg's review.

[gnprice]

Thanks @shivanshsharma13, and thanks @chrisbobbe for the previous reviews!

Generally this looks good; comments below.

[gnprice]

nit: separate with blank line

[gnprice]

nit: nest this inside the haveSameRecipient group

[gnprice]

Suggested change

	doTest('same non-letter chars', '嗎', '嗎', true);
	doTest('same non-cased chars', '嗎', '嗎', true);

Calling this character "non-letter" is confusing, because Unicode in fact classifies it in its "Letter" category, specifically "Lo (Letter, Other)":

$ unicode --long 嗎
U+55CE CJK UNIFIED IDEOGRAPH-55CE
UTF-8: e5 97 8e UTF-16BE: 55ce Decimal: 嗎 Octal: \052716
嗎
Category: Lo (Letter, Other); East Asian width: W (wide)
Unicode block: 4E00..9FFF; CJK Unified Ideographs
Bidi: L (Left-to-Right)

(By comparison, cased letters like a and A are in categories "Ll (Letter, Lowercase)" and "Lu (Letter, Uppercase)" respectively.)

[gnprice]

Suggested change

	doTest('same letters, different diacritics', 'ma', 'mǎ', false);
	doTest('having different non-letter chars', 'ma 嗎', 'mǎ 馬', false);
	doTest('same letters, different diacritics', 'ma', 'mǎ', false);
	doTest('having different non-letter chars', '嗎', '馬', false);

(plus the renaming above)

Otherwise the last test doesn't really exercise the comparison of the two sinographs — the two names are already going to be different because of the "a" vs "ǎ".