Replace PDF mode-line with page numbers

[elken]

• Dirty impl but does the job

Based on #2

Signed-off-by: Ellis Kenyo me@elken.dev

[rougier]

Great ! In fact, you can define your own nano-modeline-pdf-mode and add it to the list with the test you've already written, no need to tweak the defaullt mode I think.

[elken]

Good point. I shouldn't be hacking before bed! :)

[jacksonbenete]

Since this PR showed up first, I think there is no need to pull mine. I would like to ask some questions, as I'm new to Lisp and this will help me to improve if you don't mind.

It was my implementation:

(defun nano-modeline-pdf-view-mode-p ()
  "Return true if pdf-view-mode is the major mode."
  (if (derived-mode-p 'pdf-view-mode) t nil))

(defun nano-modeline-pdf-view-mode ()
      (let ((buffer-name (format-mode-line "%b"))
        (mode-name   (format-mode-line "%m"))
        (branch      (vc-branch))
        (page-number (concat
              (number-to-string (pdf-view-current-page)) "/"
              (or (ignore-errors
                (number-to-string (pdf-cache-number-of-pages)))
                  "???"))))
    (nano-modeline-compose (nano-modeline-status)
                   buffer-name
                   (concat "(" mode-name
                       (if branch (concat ", "
                              (propertize branch 'face 'italic)))
                       ")" )
                   page-number)))

It's not as beauty as yours, but I didn't see the need to define a function for counting the page-number since it will not be used anywhere else in the code. You think it's better to do that way to keep the let definitions smaller?

It's indeed way more readable, but I learned that let is there for avoiding defining functions of procedures not used elsewhere. So I'm not sure.

Also, I didn't used eval to get the page-numbers, I don't know why I should use in this case. Sometimes I read using eval is better, sometimes I read it should be avoided. At this point I prefer to avoid if the code is working without it. lol Why eval use is needed in this case?

I also didn't (concat " P" to keep it minimalist like the default-modeline. If you're in pdf-mode and there are numbers showing up you should already know that they refer to page number. The screenshot of how it was show is in #2 .

Also, I kept the rest of the function exactly equal the default function, because well... the pdf could be in a repository, so I don't see why I would not keep branch to be shown in the primary parameter...

I didn't PR because I'm not sure the status should be " RO " as the pdf is not really a read-only filetype. But maybe that's too pedantic of me. lol

[rougier]

@jacksonbenete I'll merge this PR and then you can modify the pdf-view-mode. I agree with your explanation. Also, the whole function will be evaled when we set the modeline so we probably do not need the val as you explain.

[rougier]

@jacksonbenete Can propose your PR now?

[rougier]

Forgot the "RO" part. As you explained and since the PDF is modifiable, it's probably better to use "RW"

[jacksonbenete]

I will send the PR in some minutes. Sorry for taking too long to answer I'm in another time-zone.