Elisp function to remove commas in the region

Question

I'm learning writing Elisp functions to perform small tasks and get to a place where executing a function on the region will replace it with the value returned by the function.

Below is a short function that aims to split the region text by commas and and concatenate the resulting substrings.

The code fails with Wrong type argument, characterp etc. After fixing this function, how can it be modified so that the string stored in final replaces the region?

    (defun myconcat()
      "Split selected buffer and concatenate all words."
      (interactive)
      (progn 
        (setq x (buffer-substring (mark) (point)))
        (setq final "")
        (setq list (split-string x ","))
        (while list
          (setq final (concat final y)))))

EDIT: Added image from running function. Strangely, the function seems to be working on an old selected buffer, but not what I currently selected.

EDIT: Another image with running myconcat after relaunching emacs

Answer 1

Your question isn't a great one for this site. Instead of "Please debug this for me" you should ask a simple, specific how-to question.

Anyway, this will get you started.

(defun myconcat(start end)
  "Split region on `,' chars and concatenate the substrings."
  (interactive "r")
  (let* ((x      (buffer-substring start end))
         (final  "")
         (y      (split-string x ",")))
    (while y
      (setq final  (concat final (car y))
            y      (cdr y)))
    (message "FINAL: %S" final)
    (delete-region start end)
    (insert final)
    final))

I'm assuming you really want to replace the active region (the selected text), not the whole buffer. You say "selected buffer", which doesn't mean anything (unless the whole buffer is selected).

If you really want to replace the whole buffer by the selected text with its , chars removed, then just replace (delete start end) by (delete (point-min) (point-max)) in the code here.

Things to note:

1. You never defined y. Presumably your vars list and y were meant to be the same.

2. No need for progn. The body of a function is an implicit progn.

3. Don't use global vars (especially undeclared - no defvar). Use let or let*.

4. Use the interactive spec to give you the region limits.

5. split-string returns a list of substrings. You need to iterate over that list, which means provide a substring to concat in each iteration.

6. Your loop never diminished the list of substrings.

7. You never deleted the original region or inserted the calculated text.

This is a bit better - the above tried to retain as much as possible of your approach, so you could more easily see the fixes.

(defun myconcat(start end)
  "Split region on `,' chars and concatenate the substrings."
  (interactive "r")
  (let ((final  ""))
    (dolist (str  (split-string (buffer-substring start end) ",")) 
      (setq final  (concat final str)))
    (message "FINAL: %S" final)
    (delete-region start end)
    (insert final)
    final))

The answer above was accepted. But if what's wanted is really just to remove the comma chars (,) in the region then there are better ways. Here's one:

(defun remove-commas-in-region (start end)
  "Remove the commas in the region."
  (interactive "r")
  (save-excursion
    (save-restriction
      (narrow-to-region start end)
      (goto-char 1)
      (while (search-forward "," nil t) (replace-match "")))))