Re: [PATCH 2/2] pretty: add '%aA' to show domain-part of email addresses

[Andy Koppe <andy.koppe@gmail.com>]

On 27/10/2023 19:40, Kousik Sanagavarapu wrote:
> Liam Beguin <liambeguin@gmail.com> wrote:
>> @@ -808,6 +808,17 @@ static size_t format_person_part(struct strbuf *sb, char part,
>>   		strbuf_add(sb, mail, maillen);
>>   		return placeholder_len;
>>   	}
>> +	if (part == 'a' || part == 'A') {	/* domain-part */
>> +		const char *at = memchr(mail, '@', maillen);
>> +		if (at) {
>> +			at += 1;
>> +			maillen -= at - mail;
>> +			strbuf_add(sb, at, maillen);
>> +		} else {
>> +			strbuf_add(sb, mail, maillen);
>> +		}
>> +		return placeholder_len;
>> +	}
>>   
>>   	if (!s.date_begin)
>>   		goto skip;
> 
> So, if we have a domain-name, we grab it, else (the case where we don't
> have '@') we grab it as-is. Looks good.

I'm not sure that this is the right way to handle a missing '@' here 
actually, because %al already returns the whole email field in that 
case, which makes sense as the likes of the 'mail' command would 
interpret it as a local username.

And if someone was going to use %al and the new specifier together to 
format the parts of the email field differently, they probably wouldn't 
want the field to appear twice.

Therefore I think it would be more appropriate to expand to nothing in 
that case. Tools that consume this output would already need to be able 
to deal with the empty case, as it could also happen if there's a single 
'@' at the end of the email field, or if the field is empty.

Regards,
Andy