emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: html2text-remove-tags documentation


From: Andreas Röhler
Subject: Re: html2text-remove-tags documentation
Date: Wed, 27 Jul 2011 08:51:16 +0200
User-agent: Mozilla/5.0 (X11; U; Linux i686; de; rv:1.9.2.18) Gecko/20110616 SUSE/3.1.11 Thunderbird/3.1.11

Am 27.07.2011 03:43, schrieb Deniz Dogan:
On 2011-07-24 11:11, Deniz Dogan wrote:
This is the definition of `html2text-remove-tags' in
lisp/gnus/html2text.el:

(defun html2text-remove-tags (tag-list)
"Removes the tags listed in the list `html2text-remove-tag-list'.
See the documentation for that variable."
(interactive)
(dolist (tag tag-list)
(goto-char (point-min))
(while (re-search-forward (format "\\(</?%s[^>]*>\\)" tag) (point-max) t)
(delete-region (match-beginning 0) (match-end 0)))))

As you can see, the documentation is clearly incorrect. The function
removes the tags in TAG-LIST, not `html2text-remove-tag-list'.
Furthermore, the function is interactive which doesn't make sense the
way it's written now.

So what should we do about this function?

I suggest we make TAG-LIST optional and defaulted to
`html2text-remove-tag-list', i.e., (or tag-list
html2text-remove-tag-list). If called interactively, TAG-LIST should be
a space-or-comma-separated string of tags to remove.

Such a definition could be:

(defun html2text-remove-tags (&optional tag-list)
"Remove the tags in TAG-LIST.
If TAG-LIST is nil, use `html2text-remove-tag-list'.
If called interactively, "
(interactive "MTags to remove: ")
(setq tag-list (if (called-interactively-p 'any)
(split-string tag-list "[ ,]" t)
(or tag-list html2text-remove-tag-list)))
(dolist (tag tag-list)
(goto-char (point-min))
(while (re-search-forward (format "\\(</?%s[^>]*>\\)" tag) (point-max) t)
(delete-region (match-beginning 0) (match-end 0)))))

This definition would not break any existing code as far as I can tell
and both fixes and adds functionality.

What do you think?


Bumping this hoping to get it in before the "hard" feature freeze.

Deniz



Hi Deniz,


as the argument is mandatory, it will not take the wrong thing.
`html2text-remove-tag-list' is just a proposal for proper use. You may give any appropriate list to work with.

You could enhance the docu maybe writing:

"Removes the tags listed in TAG-LIST, see `html2text-remove-tag-list' for example."

Also it might be useful IMHO making it restrict to active region, introducing something like:

  (let ((beg (cond (beg)
                           ((region-active-p)
                            (region-beginning))
                           (t (point-min))))
                (end (cond (end (copy-marker end))
                           ((region-active-p)
                            (copy-marker (region-end)))
                           (t (copy-marker (point-max))))))
etc.


Thanks,

Andreas




reply via email to

[Prev in Thread] Current Thread [Next in Thread]