Message search #3664

richrd · 2020-01-03T20:39:09Z

This is mainly similar to #2627 except it's all on vue.

What this does:

Adds a search field to the channel and query header bars (on mobile a search icon is shown, clicking it opens the search field as a dropdown)
Pressing enter in the field runs the search and navigates the user to the search results page (the url contains the search terms)
The search result page allows the search to be changed and run again
The results show max 100 messages at once and older messages can be loaded with the "Show older messages" button
The backend db search is just a simple LIKE %keyword%. This might require some improvement to get more relevant results

TODO:

Channels with upper case letters in the name cannot be searched

Things to improve after this MVP:

Highlight the search keyword in each search result message
Append new results when loading more messages instead of replacing the old results
Ability to jump back in history to a specific message by clicking on a search result

xPaw

We do need some extra UI to allow searching all channels on network, or even all networks.

Thinking about it in the scope of this PR because it may require a different archicture before we move further.

client/css/style.css

xPaw · 2020-01-06T08:58:45Z

src/plugins/messageStorage/sqlite.js

 	canProvideMessages() {
 		return this.isEnabled;
 	}
 }

 module.exports = MessageStorage;
+
+function parseRowToMessage(row) {


There's resolve in getMessages that should re-use this function (minus setting the id).

That being said, we need to have global ids somehow stored in sqlite because when we will want to dynamically load history from sqlite in channel history, we will not be able to assign incremental ids to them as it will be out of order.

src/plugins/messageStorage/sqlite.js

xPaw · 2020-01-06T09:00:55Z

client/components/Windows/SearchResults.vue

+									:message="message"
+								/>
+								<Message
+									:key="message.time"


I had this happen:

[Vue warn]: Duplicate keys detected: '2019-12-27T18:15:45.232Z'. This may cause an update error.

Also, context menus need fixing as they rely on activeChannel and that breaks context menus in search:

vue.js:105 TypeError: Cannot read property 'network' of null at VueComponent.openUserContextMenu

I'm getting duplicate key errors just by running a single search. There's no guarantee that the dates are unique and I indeed have two messages with the exact same date in my history. I don't know what to do about that since we don't have unique message IDs :(

xPaw · 2020-01-06T09:05:30Z

client/components/Windows/SearchResults.vue

+							aria-relevant="additions"
+						>
+							<template v-for="(message, id) in messages">
+								<DateMarker


This might be too noisy, I tried searching something and almost half of the results were date markers.

src/plugins/messageStorage/sqlite.js

src/server.js

client/components/MessageSearchForm.vue

client/components/Windows/SearchResults.vue

src/client.js

xPaw · 2020-03-24T11:55:14Z

src/client.js

@@ -501,6 +501,11 @@ Client.prototype.more = function(data) {
 	};
 };

+Client.prototype.search = function(query) {
+	const messageStorage = this.messageStorage.find((s) => s.canProvideMessages());
+	return messageStorage.search(query);


Need to handle the case when sqlite is disabled.

We still need some kind of handling (and probably hiding the search UI) if sqlite is disabled OR if user has logs disabled.

xPaw · 2020-03-24T11:59:33Z

src/plugins/messageStorage/sqlite.js

+		const params = [`%${query.searchTerm}%`];
+
+		if (query.searchNicks) {
+			select += ' OR json_extract(msg, "$.from.nick") LIKE ?)';


Shouldn't this be AND if you want to search that a user has said?

xPaw · 2020-03-24T12:04:32Z

Clicking on a username or channel throws Cannot destructure property 'network' of 'this.$store.state.activeChannel' as it is null.
I think searching for something else that returns same message as currently visible throws this warning: Duplicate keys detected: '2020-03-18T01:16:03.495Z'. This may cause an update error.
Is 'show more messages' supposed to replace current messages instead of prepending to the top?
The search box on default theme stands out and doesn't match any other existing fields color wise.
Pagination is off by one and includes 1 message from previous page.

richrd · 2020-04-24T10:37:15Z

1. Clicking on a username or channel throws `Cannot destructure property 'network' of 'this.$store.state.activeChannel' as it is null.`

2. I think searching for something else that returns same message as currently visible throws this warning: `Duplicate keys detected: '2020-03-18T01:16:03.495Z'. This may cause an update error.`

3. Is 'show more messages' supposed to replace current messages instead of prepending to the top?

4. The search box on default theme stands out and doesn't match any other existing fields color wise.

5. Pagination is off by one and includes 1 message from previous page.

Points 1, 2, 3 and 5 should be fixed now. Clicking show more adds messages to the top and keeps scroll position. Point 1 still needs testing but seems to work ok for me.
I'm not yet sure of the best way to style the search box on default theme.

Co-Authored-By: Pavel Djundik <xPaw@users.noreply.github.com>

xPaw · 2020-05-09T16:57:17Z

Do we want the 'show more messages' to automatically load when scrolling up? It doesn't at the moment.
For the sake of styling, should we also use the search icon on desktop? That way the search input won't stand out, and when focused we can make it stand out with styles.
Clicking a channel in message produces this:

vue.js:108 TypeError: Cannot read property 'network' of null
    at Object.<anonymous> (store.js:161)
    at VueComponent.onClick (InlineChannel.vue:17)

I think because there is no 'activeChannel'.

client/js/autocompletion-custom.js

xPaw · 2020-05-09T17:00:39Z

client/js/helpers/contextMenu.js

@@ -170,10 +170,12 @@ export function generateChannelContextMenu($root, channel, network) {
 }

 export function generateUserContextMenu($root, channel, network, user) {
-	const currentChannelUser = channel.users.find((u) => u.nick === network.nick) || {};
+	const currentChannelUser = channel


(channel && channel.users.find((u) => u.nick === network.nick)) || {} is slightly cleaner

fnutt · 2020-05-21T06:04:01Z

Do we want the 'show more messages' to automatically load when scrolling up? It doesn't at the moment.

For the sake of styling, should we also use the search icon on desktop? That way the search input won't stand out, and when focused we can make it stand out with styles.

Yes to both 1. and 2. please. It would be more consistent with the rest of the client.

Please also add this to the context menus so it is easy to find (e.g. I assume it's normal behaviour to right click a channel in the network-/channel list to search in it).

cottongin · 2020-07-23T20:34:57Z

Some suggestions/comments:

Can you add a way to search directly from the input form? e.g. /search query
I also like the suggestion to make the desktop client-view utilize the search icon and overflow into a separate window like @ already does for mentions. Though I have no strong feelings about it.
I feel like search results should open into their own window/view like other similar features (/banlist, /ignorelist, etc). As it stands now it is not immediately obvious how to "get back" to the channel you searched from. Edit: This would also enable you to return to the search results easily without searching the channel again.
It seems channel activity messages (joins/parts/quits/topic/modes) are ignored when searching, can an option be added to include these? This would be useful for searching for who set a ban in a channel, for example.
Similar to 4, a context option would be nice (like grep -C 5, for example) to also include additional context surrounding a search query.

Edit: I should have mentioned before all of this that I love this PR and it is working great for me. The above comments are mostly nit-picking!

vvug · 2020-09-04T12:27:30Z

Thanks for this, it's great! Will it land in time for 4.3.0?

Mstrodl · 2021-01-21T17:14:41Z

@richrd Is there anything specific needed (help-wise, I mean)? From what I saw from the comments above, it seems to be working well for other people.

fnutt · 2021-01-22T08:22:56Z

@richrd Is there anything specific needed (help-wise, I mean)? From what I saw from the comments above, it seems to be working well for other people.

At least a rebase has to be done. "Channels with upper case letters in the name cannot be searched" seems to be on the TODO-list, and @xPaw has stated quite a few things that has to be fixed.

There was also some questions about how to filter the search, e.g. search just messages from user X, older than X etc. If this is added the PR may have to be redone regarding the implementation.

Personally I would love to see a rebase, get the channel upper case letters bug fixed, fix @xPaw's review comments, and then merged to master. If we want to add search filter and other stuff later, someone can redo the implementation if needed.

Another option would be to just rebase the PR. Then we can at least use it by running TL from source and merge this PR locally.

richrd force-pushed the richrd/message-search branch 2 times, most recently from 93f9c98 to b088786 Compare January 3, 2020 22:00

richrd changed the title ~~Message search WIP~~ Message search Jan 4, 2020

richrd force-pushed the richrd/message-search branch from 173d46d to f13543b Compare January 4, 2020 18:26

xPaw requested changes Jan 6, 2020

View reviewed changes

xPaw added the Type: Feature Tickets that describe a desired feature or PRs that add them to the project. label Jan 8, 2020

richrd force-pushed the richrd/message-search branch from f13543b to c2f1b20 Compare February 10, 2020 18:21

richrd force-pushed the richrd/message-search branch 2 times, most recently from a7195ac to c28a880 Compare March 7, 2020 13:07

xPaw reviewed Mar 24, 2020

View reviewed changes

richrd force-pushed the richrd/message-search branch 3 times, most recently from d257143 to 9aa5864 Compare April 17, 2020 21:32

richrd force-pushed the richrd/message-search branch from 9aa5864 to 99cfba6 Compare April 20, 2020 14:59

xPaw linked an issue Apr 23, 2020 that may be closed by this pull request

Message search #109

Closed

dgw mentioned this pull request Apr 23, 2020

Message search #109

Closed

richrd force-pushed the richrd/message-search branch from 1ca52b9 to bc117e2 Compare April 24, 2020 09:30

richrd and others added 7 commits April 26, 2020 22:39

Message search WIP.

0f3c292

Disable searching nicks.

2591ae9

Fix toggling search on mobile.

28c740a

Use ellipsis

4ba458b

Co-Authored-By: Pavel Djundik <xPaw@users.noreply.github.com>

Use ellipsis

8864431

Co-Authored-By: Pavel Djundik <xPaw@users.noreply.github.com>

Search improvements.

9a1fb0c

Append new results and keep scroll position.

5d017b0

richrd force-pushed the richrd/message-search branch from bc117e2 to af86d85 Compare April 26, 2020 19:39

xPaw added this to the 4.2.0 milestone May 9, 2020

xPaw reviewed May 9, 2020

View reviewed changes

client/js/autocompletion-custom.js Outdated Show resolved Hide resolved

xPaw reviewed May 9, 2020

View reviewed changes

Fix context menus in search results.

b4d02c3

richrd force-pushed the richrd/message-search branch from af86d85 to b4d02c3 Compare June 3, 2020 11:05

Fix searching channels with uppercase letters in name.

f323653

xPaw assigned richrd Jun 22, 2020

xPaw removed this from the 4.2.0 milestone Jul 27, 2020

richrd added the help wanted Tickets the community can help us with, by either answering questions or sending us PRs. label Oct 1, 2020

Nachtalb mentioned this pull request Apr 3, 2021

ToDo List: Nachtalb/thelounge#1

Closed

9 tasks

Nachtalb mentioned this pull request Apr 13, 2021

Message Search: Re-Rebase + Fixes / Adjustments #4197

Merged

MaxLeiter merged commit 283ef44 into master May 3, 2021

MaxLeiter deleted the richrd/message-search branch May 3, 2021 23:18

fnutt mentioned this pull request May 4, 2021

Message search improvements #4209

Open

MaxLeiter mentioned this pull request Nov 23, 2021

Remove scripts/changelog.js in favor of GitHub's automatic changelog generation #4370

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Message search #3664

Message search #3664

richrd commented Jan 3, 2020 •

edited

xPaw left a comment

xPaw Jan 6, 2020

xPaw Jan 6, 2020

xPaw Jan 6, 2020

richrd Apr 13, 2020

xPaw Jan 6, 2020

xPaw Mar 24, 2020

xPaw May 9, 2020

xPaw Mar 24, 2020

xPaw commented Mar 24, 2020 •

edited

richrd commented Apr 24, 2020 •

edited

xPaw commented May 9, 2020

xPaw May 9, 2020

fnutt commented May 21, 2020 •

edited

cottongin commented Jul 23, 2020 •

edited

vvug commented Sep 4, 2020

Mstrodl commented Jan 21, 2021

fnutt commented Jan 22, 2021 •

edited

Message search #3664

Message search #3664

Conversation

richrd commented Jan 3, 2020 • edited

xPaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xPaw commented Mar 24, 2020 • edited

richrd commented Apr 24, 2020 • edited

xPaw commented May 9, 2020

Choose a reason for hiding this comment

fnutt commented May 21, 2020 • edited

cottongin commented Jul 23, 2020 • edited

vvug commented Sep 4, 2020

Mstrodl commented Jan 21, 2021

fnutt commented Jan 22, 2021 • edited

richrd commented Jan 3, 2020 •

edited

xPaw commented Mar 24, 2020 •

edited

richrd commented Apr 24, 2020 •

edited

fnutt commented May 21, 2020 •

edited

cottongin commented Jul 23, 2020 •

edited

fnutt commented Jan 22, 2021 •

edited