~emersion/public-inbox

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH gamja] Linkify channel names

Details
Message ID
<20210529135019.2071604-1-me@tomlebreux.com>
DKIM signature
pass
Download raw message
Patch: +75 -9
---
As discussed, this creates links for channel names and on click it will
automatically join the channel and switch to the buffer.

I am not sure if there is a better way to detect channel names, and I
also wasn't sure if we wanted to detect all channel (# and &).
 components/app.js           | 17 ++++++++++++++-
 components/buffer-header.js |  6 +++++-
 components/buffer.js        | 19 +++++++++++++----
 lib/linkify.js              | 42 ++++++++++++++++++++++++++++++++++---
 4 files changed, 75 insertions(+), 9 deletions(-)

diff --git a/components/app.js b/components/app.js
index 38d3cb46cc9b..5c482c3e0775 100644
--- a/components/app.js
+++ b/components/app.js
@@ -197,6 +197,7 @@ export default class App extends Component {
		this.toggleBufferList = this.toggleBufferList.bind(this);
		this.toggleMemberList = this.toggleMemberList.bind(this);
		this.handleComposerSubmit = this.handleComposerSubmit.bind(this);
		this.handleChannelClick = this.handleChannelClick.bind(this);
		this.handleNickClick = this.handleNickClick.bind(this);
		this.autocomplete = this.autocomplete.bind(this);
		this.handleBufferScrollTop = this.handleBufferScrollTop.bind(this);
@@ -891,6 +892,16 @@ export default class App extends Component {
		this.connect(connectParams);
	}

	handleChannelClick(channel) {
		var netID = getActiveNetworkID(this.state);
		var buf = getBuffer(this.state, { network: netID, name: channel});
		if (buf) {
			this.switchBuffer(buf.id);
		} else {
			this.open(channel);
		}
	}

	handleNickClick(nick) {
		this.open(nick);
	}
@@ -1261,6 +1272,7 @@ export default class App extends Component {
						network=${activeNetwork}
						isBouncer=${isBouncer}
						bouncerNetwork=${activeBouncerNetwork}
						onChannelClick=${this.handleChannelClick}
						onClose=${() => this.close(activeBuffer)}
						onJoin=${() => this.handleJoinClick(activeBuffer.network)}
						onAddNetwork=${this.handleAddNetworkClick}
@@ -1367,7 +1379,10 @@ export default class App extends Component {
				onScrollTop=${this.handleBufferScrollTop}
			>
				<section id="buffer" ref=${this.buffer}>
					<${Buffer} buffer=${activeBuffer} onNickClick=${this.handleNickClick}/>
					<${Buffer}
						buffer=${activeBuffer}
						onChannelClick=${this.handleChannelClick}
						onNickClick=${this.handleNickClick}/>
				</section>
			</>
			${memberList}
diff --git a/components/buffer-header.js b/components/buffer-header.js
index 62c80651bd42..a43b1ed7feea 100644
--- a/components/buffer-header.js
+++ b/components/buffer-header.js
@@ -20,6 +20,10 @@ function NickStatus(props) {
}

export default function BufferHeader(props) {
	function handleChannelClick(event, channel) {
		event.preventDefault();
		props.onChannelClick(channel);
	}
	function handleCloseClick(event) {
		event.preventDefault();
		props.onClose();
@@ -70,7 +74,7 @@ export default function BufferHeader(props) {
			break;
		}
	} else if (props.buffer.topic) {
		description = linkify(stripANSI(props.buffer.topic));
		description = linkify(stripANSI(props.buffer.topic), handleChannelClick);
	} else if (props.buffer.who) {
		var who = props.buffer.who;

diff --git a/components/buffer.js b/components/buffer.js
index b92e5805bbf2..f3042291de50 100644
--- a/components/buffer.js
+++ b/components/buffer.js
@@ -40,6 +40,11 @@ function Timestamp({ date, url }) {
}

class LogLine extends Component {
	handleChannelClick(event, channel) {
		event.preventDefault();
		this.props.onChannelClick(channel);
	}

	shouldComponentUpdate(nextProps) {
		return this.props.message !== nextProps.message;
	}
@@ -47,6 +52,7 @@ class LogLine extends Component {
	render() {
		var msg = this.props.message;

		var onChannelClick = this.handleChannelClick.bind(this);
		var onNickClick = this.props.onNickClick;
		function createNick(nick) {
			return html`
@@ -65,7 +71,7 @@ class LogLine extends Component {
			if (ctcp) {
				if (ctcp.command == "ACTION") {
					lineClass = "me-tell";
					content = html`* ${createNick(msg.prefix.name)} ${linkify(stripANSI(ctcp.param))}`;
					content = html`* ${createNick(msg.prefix.name)} ${linkify(stripANSI(ctcp.param), onChannelClick)}`;
				} else {
					content = html`
						${createNick(msg.prefix.name)} has sent a CTCP command: ${ctcp.command} ${ctcp.param}
@@ -77,7 +83,7 @@ class LogLine extends Component {
				if (msg.command == "NOTICE") {
					prefix = suffix = "-";
				}
				content = html`${prefix}${createNick(msg.prefix.name)}${suffix} ${linkify(stripANSI(text))}`;
				content = html`${prefix}${createNick(msg.prefix.name)}${suffix} ${linkify(stripANSI(text), onChannelClick)}`;
			}

			if (msg.isHighlight) {
@@ -118,7 +124,7 @@ class LogLine extends Component {
		case "TOPIC":
			var topic = msg.params[1];
			content = html`
				${createNick(msg.prefix.name)} changed the topic to: ${linkify(stripANSI(topic))}
				${createNick(msg.prefix.name)} changed the topic to: ${linkify(stripANSI(topic), onChannelClick)}
			`;
			break;
		case irc.RPL_MOTD:
@@ -242,7 +248,12 @@ export default class Buffer extends Component {
			prevDate = date;

			children.push(html`
				<${LogLine} key=${"msg-" + msg.key} message=${msg} buffer=${buf} onNickClick=${this.props.onNickClick}/>
				<${LogLine}
					key=${"msg-" + msg.key}
					message=${msg}
					buffer=${buf}
					onChannelClick=${this.props.onChannelClick}
					onNickClick=${this.props.onNickClick}/>
			`);
		});

diff --git a/lib/linkify.js b/lib/linkify.js
index c9697cd1bd4a..269787767e0a 100644
--- a/lib/linkify.js
+++ b/lib/linkify.js
@@ -1,12 +1,46 @@
import { anchorme, html } from "./index.js";

export default function linkify(text) {
function linkifyChannel(text, transformChannel) {
	var children = [];
	const channelRegex = /(^|\s+)(#[^\s]+)/gid;
	let match;

	var last = 0;
	while ((match = channelRegex.exec(text)) !== null) {
		const [_, spaces, channel] = match;

		const start = match.index + spaces.length;
		children.push(text.substring(last, start));

		if (transformChannel) {
			children.push(transformChannel(channel));
		} else {
			children.push(channel);
		}

		last = match.index + spaces.length + channel.length;
	}
	children.push(text.substring(last));

	return children;
}

export default function linkify(text, onChannelClick) {
	const transformChannel = (channel) => {
		return html`
			<a
				href="irc:///${channel}"
				onClick=${(ev) => onChannelClick(ev, channel)}
			>${channel}</a>`;
	}

	var links = anchorme.list(text);

	var children = [];
	var last = 0;
	links.forEach((match) => {
		children.push(text.substring(last, match.start));
		const prefix = text.substring(last, match.start)
		children.push(...linkifyChannel(prefix, transformChannel));

		var proto = match.protocol || "https://";
		if (match.isEmail) {
@@ -22,7 +56,9 @@ export default function linkify(text) {

		last = match.end;
	});
	children.push(text.substring(last));

	const suffix = text.substring(last)
	children.push(...linkifyChannel(suffix, transformChannel));

	return children;
}
-- 
2.31.1
Details
Message ID
<JS-HI4AkIOiKLYAG9_qQ3s2dzM9nr3Zl_ePaKuVOsDjVL1nhw9unsf2fbHpkkEyK_35zBXaA0nCE_1FyshRr73HM_gcHot7WkUWEnLTUs0U=@emersion.fr>
In-Reply-To
<20210529135019.2071604-1-me@tomlebreux.com> (view parent)
DKIM signature
pass
Download raw message
On Saturday, May 29th, 2021 at 3:50 PM, Tom Lebreux <me@tomlebreux.com> wrote:

> As discussed, this creates links for channel names and on click it will
> automatically join the channel and switch to the buffer.

Thanks a lot for your patch! Overall it looks good to me, a few minor comments
below.

> I am not sure if there is a better way to detect channel names, and I
> also wasn't sure if we wanted to detect all channel (# and &).

I think only detecting # is fine, since that's what everybody uses in
practice. It's no big deal if _some_ channel names aren't linkified.

>  components/app.js           | 17 ++++++++++++++-
>  components/buffer-header.js |  6 +++++-
>  components/buffer.js        | 19 +++++++++++++----
>  lib/linkify.js              | 42 ++++++++++++++++++++++++++++++++++---
>  4 files changed, 75 insertions(+), 9 deletions(-)
>
> diff --git a/components/app.js b/components/app.js
> index 38d3cb46cc9b..5c482c3e0775 100644
> --- a/components/app.js
> +++ b/components/app.js
> @@ -197,6 +197,7 @@ export default class App extends Component {
>  		this.toggleBufferList = this.toggleBufferList.bind(this);
>  		this.toggleMemberList = this.toggleMemberList.bind(this);
>  		this.handleComposerSubmit = this.handleComposerSubmit.bind(this);
> +		this.handleChannelClick = this.handleChannelClick.bind(this);
>  		this.handleNickClick = this.handleNickClick.bind(this);
>  		this.autocomplete = this.autocomplete.bind(this);
>  		this.handleBufferScrollTop = this.handleBufferScrollTop.bind(this);
> @@ -891,6 +892,16 @@ export default class App extends Component {
>  		this.connect(connectParams);
>  	}
>
> +	handleChannelClick(channel) {
> +		var netID = getActiveNetworkID(this.state);
> +		var buf = getBuffer(this.state, { network: netID, name: channel});

Style: missing space between `channel` and `}`.

> +		if (buf) {
> +			this.switchBuffer(buf.id);
> +		} else {
> +			this.open(channel);
> +		}
> +	}
> +
>  	handleNickClick(nick) {
>  		this.open(nick);
>  	}
> @@ -1261,6 +1272,7 @@ export default class App extends Component {
>  						network=${activeNetwork}
>  						isBouncer=${isBouncer}
>  						bouncerNetwork=${activeBouncerNetwork}
> +						onChannelClick=${this.handleChannelClick}
>  						onClose=${() => this.close(activeBuffer)}
>  						onJoin=${() => this.handleJoinClick(activeBuffer.network)}
>  						onAddNetwork=${this.handleAddNetworkClick}
> @@ -1367,7 +1379,10 @@ export default class App extends Component {
>  				onScrollTop=${this.handleBufferScrollTop}
>  			>
>  				<section id="buffer" ref=${this.buffer}>
> -					<${Buffer} buffer=${activeBuffer} onNickClick=${this.handleNickClick}/>
> +					<${Buffer}
> +						buffer=${activeBuffer}
> +						onChannelClick=${this.handleChannelClick}
> +						onNickClick=${this.handleNickClick}/>
>  				</section>
>  			</>
>  			${memberList}
> diff --git a/components/buffer-header.js b/components/buffer-header.js
> index 62c80651bd42..a43b1ed7feea 100644
> --- a/components/buffer-header.js
> +++ b/components/buffer-header.js
> @@ -20,6 +20,10 @@ function NickStatus(props) {
>  }
>
>  export default function BufferHeader(props) {
> +	function handleChannelClick(event, channel) {
> +		event.preventDefault();

Can we move these preventDefault() calls down to the linkify function?

> +		props.onChannelClick(channel);
> +	}
>  	function handleCloseClick(event) {
>  		event.preventDefault();
>  		props.onClose();
> @@ -70,7 +74,7 @@ export default function BufferHeader(props) {
>  			break;
>  		}
>  	} else if (props.buffer.topic) {
> -		description = linkify(stripANSI(props.buffer.topic));
> +		description = linkify(stripANSI(props.buffer.topic), handleChannelClick);
>  	} else if (props.buffer.who) {
>  		var who = props.buffer.who;
>
> diff --git a/components/buffer.js b/components/buffer.js
> index b92e5805bbf2..f3042291de50 100644
> --- a/components/buffer.js
> +++ b/components/buffer.js
> @@ -40,6 +40,11 @@ function Timestamp({ date, url }) {
>  }
>
>  class LogLine extends Component {
> +	handleChannelClick(event, channel) {
> +		event.preventDefault();
> +		this.props.onChannelClick(channel);
> +	}
> +
>  	shouldComponentUpdate(nextProps) {
>  		return this.props.message !== nextProps.message;
>  	}
> @@ -47,6 +52,7 @@ class LogLine extends Component {
>  	render() {
>  		var msg = this.props.message;
>
> +		var onChannelClick = this.handleChannelClick.bind(this);
>  		var onNickClick = this.props.onNickClick;
>  		function createNick(nick) {
>  			return html`
> @@ -65,7 +71,7 @@ class LogLine extends Component {
>  			if (ctcp) {
>  				if (ctcp.command == "ACTION") {
>  					lineClass = "me-tell";
> -					content = html`* ${createNick(msg.prefix.name)} ${linkify(stripANSI(ctcp.param))}`;
> +					content = html`* ${createNick(msg.prefix.name)} ${linkify(stripANSI(ctcp.param), onChannelClick)}`;
>  				} else {
>  					content = html`
>  						${createNick(msg.prefix.name)} has sent a CTCP command: ${ctcp.command} ${ctcp.param}
> @@ -77,7 +83,7 @@ class LogLine extends Component {
>  				if (msg.command == "NOTICE") {
>  					prefix = suffix = "-";
>  				}
> -				content = html`${prefix}${createNick(msg.prefix.name)}${suffix} ${linkify(stripANSI(text))}`;
> +				content = html`${prefix}${createNick(msg.prefix.name)}${suffix} ${linkify(stripANSI(text), onChannelClick)}`;
>  			}
>
>  			if (msg.isHighlight) {
> @@ -118,7 +124,7 @@ class LogLine extends Component {
>  		case "TOPIC":
>  			var topic = msg.params[1];
>  			content = html`
> -				${createNick(msg.prefix.name)} changed the topic to: ${linkify(stripANSI(topic))}
> +				${createNick(msg.prefix.name)} changed the topic to: ${linkify(stripANSI(topic), onChannelClick)}
>  			`;
>  			break;
>  		case irc.RPL_MOTD:
> @@ -242,7 +248,12 @@ export default class Buffer extends Component {
>  			prevDate = date;
>
>  			children.push(html`
> -				<${LogLine} key=${"msg-" + msg.key} message=${msg} buffer=${buf} onNickClick=${this.props.onNickClick}/>
> +				<${LogLine}
> +					key=${"msg-" + msg.key}
> +					message=${msg}
> +					buffer=${buf}
> +					onChannelClick=${this.props.onChannelClick}
> +					onNickClick=${this.props.onNickClick}/>
>  			`);
>  		});
>
> diff --git a/lib/linkify.js b/lib/linkify.js
> index c9697cd1bd4a..269787767e0a 100644
> --- a/lib/linkify.js
> +++ b/lib/linkify.js
> @@ -1,12 +1,46 @@
>  import { anchorme, html } from "./index.js";
>
> -export default function linkify(text) {
> +function linkifyChannel(text, transformChannel) {
> +	var children = [];
> +	const channelRegex = /(^|\s+)(#[^\s]+)/gid;

Commas are not valid in channel names.

We'll probably want to avoid linkifying punctuation at the end of the channel
name, as in "Can you join #soju?" or "Can you join our channel (it's named
#soju)". But I'm fine with keeping this as a TODO for now.

Nit: we can probably only match a single space here.

> +	let match;
> +
> +	var last = 0;
> +	while ((match = channelRegex.exec(text)) !== null) {
> +		const [_, spaces, channel] = match;
> +
> +		const start = match.index + spaces.length;
> +		children.push(text.substring(last, start));
> +
> +		if (transformChannel) {
> +			children.push(transformChannel(channel));
> +		} else {
> +			children.push(channel);
> +		}

Nit: we can probably get rid of this `if`, since `transformChannel` is always
specified.

> +		last = match.index + spaces.length + channel.length;
> +	}
> +	children.push(text.substring(last));
> +
> +	return children;
> +}
> +
> +export default function linkify(text, onChannelClick) {

Note for the future: at some point we'll probably want to support linkified
irc:// URLs as well [1]. So long-term we'll probably want to have higher-level
support for irc:// URLs, since these can refer not only to channel but also
to nicks or servers. Maybe it'd be better to just have a global app-wide click
handler that intercepts cliks on irc:// links and does the right thing.

But let's keep all of this for later.

[1]: https://todo.sr.ht/~emersion/gamja/71

> +	const transformChannel = (channel) => {

Style: can use `function` here.

> +		return html`
> +			<a
> +				href="irc:///${channel}"

We should encodeURIComponent the channel here.

> +				onClick=${(ev) => onChannelClick(ev, channel)}
> +			>${channel}</a>`;
> +	}
> +
>  	var links = anchorme.list(text);
>
>  	var children = [];
>  	var last = 0;
>  	links.forEach((match) => {
> -		children.push(text.substring(last, match.start));
> +		const prefix = text.substring(last, match.start)
> +		children.push(...linkifyChannel(prefix, transformChannel));
>
>  		var proto = match.protocol || "https://";
>  		if (match.isEmail) {
> @@ -22,7 +56,9 @@ export default function linkify(text) {
>
>  		last = match.end;
>  	});
> -	children.push(text.substring(last));
> +
> +	const suffix = text.substring(last)
> +	children.push(...linkifyChannel(suffix, transformChannel));
>
>  	return children;
>  }
> --
> 2.31.1
Reply to thread Export thread (mbox)