Subject: bug#49033: 28.0.50; [PATCH] Feature suggestion, url-cache-expiry-alist to override expire time for cache pruning

From: Lars Ingebrigtsen <[email-masked]>
Date: Tue, 15 Jun 2021 16:11:23 +0200

Alex Bochannek <[email-masked]> writes:

> +(defcustom url-cache-expiry-alist nil
> +  "Alist of URL regular expressions to override the `url-cache-expire-time'."
> +  :version "28.1"
> +  :type 'alist
> +  :group 'url-cache)

Sure, adding this makes sense to me.

> +(defun url-cache-create-url-from-file (file)

I thought this existed, but I guess not...  

> +		(let ((expire-time
> +		       (remove
> +			nil
> +			(mapcar
> +			 (lambda (alist)
> +			   (let ((key (car alist))
> +				(value (cdr alist)))
> +			     (if
> +				 (string-match
> +				  key
> +				  (url-cache-create-url-from-file file))
> +				  value)))
> +			url-cache-expiry-alist))))
> +		 (if (consp expire-time) (apply 'min expire-time) nil))
> +	       url-cache-expire-time))
>  	     now)
>  	    (delete-file file)
>  	    (setq deleted-files (1+ deleted-files))))))
>
> I also didn't really like the way the code ended up being formatted. Is
> there some guidance around splitting functions and their arguments
> across multiple lines?

Well, we never do:

> +			     (if
> +				 (string-match

But in general, we just try to keep it readable, which means not going
too far in either horizontal nor vertical directions.  (So there's
really no rules for formatting beyond that.)  Your code in this patch
generally seems to be way too vertical.

> -                 ;; Twelve hours.
> -                 (* 12 60 60))))
> +                 gravatar-cache-ttl)))

I don't mind that -- but is this really something that somebody would
want to control?  It just seemed unlikely to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no