1

I'm trying to prepend each substring of split-string with numbering (1.,2.,3.,..) then return all the substrings concatenated with a newline

(defun japanese-get-definition (dictentry)
  "Get a definition from a dictionary entry."
  (let* ((index 1)
         (definitions))
    (save-match-data
      (and (string-match "/\\(.*?\\)$" dictentry)
           (setq definitions (split-string (match-string 1 dictentry) "/" t))
           (dolist (definition definitions)
             (setq definition (concat (int-to-string index) ". " definition))
             (setq index (+ index 1)))
           (mapconcat 'identity
                      definitions
                      "\n"))
      )
    )
  )

(japanese-get-definition "職業 [しょくぎょう] /(n,adj-no) occupation/business/")

So the result would looks like

1. (n,adj-no) occupation
2. business

Currently though the function is returning nil. Is dolist the right function to use? This would probably be very easy if I just use a new list, but I'm hoping to use learn the best way to do this (which I think (could be wrong) is modifying the existing list so less memory is used)

  • 3
    Emacs Lisp is not a language where memory usage should be your primary concern. Copying lists is totally fine, and treating lists as immutable helps you write better code. – user227 May 21 '16 at 18:50
  • @lunaryorn That's fine I may switch to copying to a list but I am still extremely curious as to why mapconcat returns nil for the above. – irregular May 21 '16 at 20:52
  • It's dolist which returns nil here, but that's documented behaviour? – user227 May 21 '16 at 20:56
3
  1. The dolist can have no effect on definitions. If you remove it from your function you will see that you get a result (but not exactly what you wanted). The mapconcat is evaluated and does not return nil.

  2. The result of (setq definitions (split-string (match-string 1 dictentry) "/" t)) in your test example is ("(n,adj-no) occupation" "business").

  3. The mapconcat applied to that would give "(n,adj-no) occupation\nbusiness"

  4. dolist returns nil, so the mapconcat is never evaluated.

  5. If you wrap your dolist with (progn ... t) then the mapconcat will be evaluated.

But your main problem is thinking that dolist modifies the original list. As explained in the post you cited in your other question, you can use dolist to modify individual elements of the list but not the list itself.

For example, if the list had particular strings or lists as its elements, you could modify those individual objects. (The individual elements of the list must of course be modifiable, for you to modify them. You cannot modify numbers, for example.)

But in your case you want to modify the list itself. You want to replace a given element with a completely different value -- that is, replace it in the list. That's not the same as modifying an element (which changes the value globally). You cannot do that with dolist.

Here's one quick, ugly-hacky way to do what you want. It doesn't change your code much. It just replaces the dolist by some code that modifies definitions.

(defun japanese-get-definition (dictentry)
  "Get a definition from a dictionary entry."
  (let* ((index 1)
         (definitions))
    (save-match-data
      (and (string-match "/\\(.*?\\)$" dictentry)
           (setq definitions (split-string (match-string 1 dictentry) "/" t))
           (let ((defs  definitions))
             (while defs
               (setcar defs (concat (int-to-string index) ". " (car defs)))
               (setq index  (1+ index)
                     defs   (cdr defs)))
             t)
           (mapconcat 'identity definitions "\n")))))

This is slightly simpler, but still not very pretty:

(defun japanese-get-definition (dictentry)
  "Get a definition from a dictionary entry."
  (let* ((index       1)
         (definitions ()))
    (save-match-data
      (when (string-match "/\\(.*?\\)$" dictentry)
        (setq definitions  (split-string (match-string 1 dictentry) "/" t))
        (let ((defs  definitions))
          (while defs
            (setcar defs (concat (int-to-string index) ". " (car defs)))
            (setq index  (1+ index)
                  defs   (cdr defs))))
        (mapconcat 'identity definitions "\n")))))
| improve this answer | |
  • The explanation on each function is much appreciated! Someone on the irc channel also gave a cleaner solution (which I posted in a separate answer) that I liked a lot so I also answered the question to share it with others! – irregular May 21 '16 at 21:50
1

Thank you Drew for the explanation on what I was doing wrong. Another way to approach the problem (thanks tali713) is with a lambda in mapconcat

(defun japanese-get-definition (dictentry)
  "Get a definition from a dictionary entry."
  (let ((index 0))
    (mapconcat (lambda (str) (format "%s. %s" (incf index) str))
               (split-string dictentry "/" t)
               "\n")))

for a much cleaner solution.

| improve this answer | |
  • Yes, of course. I was trying to keep close to your original code and speak about dolist and the question of what was returning nil. This is the right way to do it: let mapconcat do everything. – Drew May 21 '16 at 22:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.