Add docview pagenumber indication

[dalanicolai]

This commit fixes the page number indication for doc-view mode, exactly in the same way as it has been done for pdf-view mode. Although most users probably use pdf-tools for reading pdf, doc-view is still very handy for various other file formats, e.g. djvu, odf and possibly epub.

This PR is an updated copy of PR #139. Please merge, it is a small but handy PR.

[duianto]

This seems to be working as expected.

Before

After

System info

spaceline-20191230.1221 GNU Emacs 27.1 (build 1, x86_64-w64-mingw32) of 2020-08-21 Windows 10 2004

[duianto]

I could probably merge this, (I've been invited as a collaborator).

But I'm not sure what the rules are about merging stuff that fails the tests. @TheBB

In this case the failed test says: https://travis-ci.org/github/TheBB/spaceline/jobs/735077592

Setting environment variables from .travis.yml
$ export EVM_EMACS=emacs-26.1-travis
$ bash -c 'echo $BASH_VERSION'
4.3.48(1)-release
before_install.1
9.51s$ curl -fsSkL https://gist.github.com/rejeep/ebcd57c3af83b049833b/raw > x.sh && source ./x.sh
before_install.2
2.73s$ evm install $EVM_EMACS --use --skip
0.05s$ cask install
Traceback (most recent call last):
  File "/home/travis/.cask/bin/cask", line 425, in <module>
    main()
  File "/home/travis/.cask/bin/cask", line 405, in main
    exec_cask(sys.argv[1:])
  File "/home/travis/.cask/bin/cask", line 354, in exec_cask
    emacs = get_cask_emacs()
  File "/home/travis/.cask/bin/cask", line 298, in get_cask_emacs
    ensure_supported_emacs(emacs)
  File "/home/travis/.cask/bin/cask", line 230, in ensure_supported_emacs
    if not is_supported_emacs(emacs):
  File "/home/travis/.cask/bin/cask", line 215, in is_supported_emacs
    return get_emacs_version(emacs) >= MIN_EMACS_VERSION
  File "/home/travis/.cask/bin/cask", line 199, in get_emacs_version
    'Could not determine the version of Emacs at {0}'.format(emacs))
ValueError: Could not determine the version of Emacs at emacs
The command "cask install" failed and exited with 1 during .

[TheBB]

Feel free to make your own judgment @duianto. In this case I think it's clear that the CI setup has deteriorated and the failure is not genuine. After all I haven't touched this repo in ages.

[duianto]

I got a suggestion for a possible fix for the test issue, but it didn't seem to work.

So let's try merging this anyway, since it seems to be unrelated to the test.

Update: Correction, the test is working: https://travis-ci.org/github/TheBB/spaceline/builds/736161567

I had restarted it with the previous config.

[duianto]

Thanks for contributing to Spaceline.

[duianto]

It didn't seem to work as expected, the mode line is blank when a pdf file is opened.

The messages buffer shows:

Error during redisplay: (eval (spaceline-ml-main)) signaled (invalid-function doc-view-current-page)

I didn't get that error when I copied over the changes manually, in the test above.

I tried using that line from the previous PRs: https://github.com/TheBB/spaceline/pull/139/files#diff-8b2b0c5ed40962c7159ad3042e9fffd4098ec59d020b9208e3d6a6a45f11f845R159

1 file changed, 1 insertion(+), 1 deletion(-)
spaceline-segments.el | 2 +-

modified   spaceline-segments.el
@@ -157,7 +157,7 @@
 Return a formated string containing the current and last page number for the
 currently displayed pdf file in `doc-view-mode'."
   (format "(%d/%d)"
-          (doc-view-current-page)
+          (eval `(doc-view-current-page))
           (doc-view-last-page-number)))

 (declare-function pdf-view-current-page 'pdf-view)

And it seems to work when applied manually, but I'm not sure I can trust the manual test since it didn't work before.

Any ideas about why it worked before pushing the change?

[TheBB]

(defmacro doc-view-current-page (&optional win)
  `(image-mode-window-get 'page ,win))

Looks like it's a macro and not a function. Probably screws things up if the modeline is compiled before doc-view is loaded, or something like that. Maybe best to replace it for now with its expansion:

(image-mode-window-get 'page nil)

[duianto]

Thanks for the suggestion. I pushed the fix.

Ok, maybe my mistake was that I just deleted the .elc file and modified the .el file. I probably have to byte-compile-file the .el file to test it correctly?

[TheBB]

Yeah that could be. But honestly there's some byte-compilation and macro-expansion shenanigans going on in Spaceline, implemented in the name of premature optimization, that probably shouldn't be there.

[duianto]

It didn't work to reproduce the blank mode line by byte compiling spaceline-segments.el, after restoring this PRs macro line: (doc-view-current-page)

Let's see if it works when melpa updates.

[duianto]

The fix worked.

[dalanicolai]

Thanks for merging! Too bad I only read this now, because I knew that it might cause a problem. It did not when I tested it but also I did not test it with byte-compiling (I do not fully understand the macro compiling issues things completely yet). However, I did notice, when I just copied the spaceline--pdfview-page-number function that there are the lines:

(declare-function pdf-view-current-page 'pdf-view)
(declare-function pdf-cache-number-of-pages 'pdf-view)

above it. Also spaceline--pdfview-page-number uses (eval(pdf-view-current-page))`. But I thought that was not really necessary for the doc-view variant and it worked without it when I tested it. Otherwise I assumed, @TheBB would immediately recognize it (I did not consider the possibility that others might help maintaining this repo).

Also I encountered a similar problem in the pdf-continuous-scroll-mode package, where after some 'research' I solved it by adding

(eval-when-compile
  (require 'pdf-view))

at the beginning of the package.

Anyway, I commented here first to offer my apologies for not mentioning these things in the PR. Of course I will try to be more careful next time and add these doubts in the PR to prevent you from wasting time on investigating such issues. A second reason is to share what I know in order to help you decide on the cleanest solution. I am trying to understand these things well also myself, but it takes time (which is very limited unfortunately).

Thanks again for merging. I got notified that Politza intents to stop maintaining pdf-tools. And I think doc-view (i.e. other formats than PDF) could profit from many pdf-tools functionalities (i.e. I hope they will get merged in the future).