[bug] `airline-position-segment` not displaying `mode-line-position`, important for some major modes

[leonardo-cavegliacurtil]

Some major modes, such as PDFView, use mode-line-position to communicate the buffer position aside from lines and columns. When using telephone-line-airline-position-segment, such values are not displayed.

The following screenshot depicts telephone-line-airline-position-segment beside telephone-line-position-segment, which is properly using mode-line-position.

---

My proposed possible solutions would be:

• parse mode-line-position to turn it into an airline-styled position segment only if a regex is matched -- e.g. /^L\d+$/, and maybe concat columns regardless
• unless the buffer has lines and columns, show mode-line-position
• make an exception for known major modes (seems like a less clean solution)

---

Related code from telephone-line-segments.el follows.

(telephone-line-defsegment* telephone-line-position-segment ()
  (telephone-line-raw
   (if (eq major-mode 'paradox-menu-mode)
       ;;Paradox fills this with position info.
       mode-line-front-space
     mode-line-position) t))

(telephone-line-defsegment* telephone-line-airline-position-segment (&optional lines columns)
  "Position segment imitating vim-airline's appearance. Optional args set padding on lines/columns."
  (let* ((l (number-to-string (if lines lines 4)))
         (c (number-to-string (if columns columns 3))))
    (if (eq major-mode 'paradox-menu-mode)
        (telephone-line-raw mode-line-front-space t)
      `((-3 "%p") ,(concat " %" l "l"
                           ":%" c (if (bound-and-true-p column-number-indicator-zero-based) "c" "C"))))))

[leonardo-cavegliacurtil]

Here is my proposed version: if it matches the regex /L\d+\s*$/, it uses the previous "airline" code; otherwise, it uses mode-line-position.

Ideally, I'd use a smarted regex like /^\s*\w+\s*L\d+\s*$/, which should match only the patterns seen in text buffers (e.g. Top L40 or 5% L40), but I can't get them to work properly.

Tested and apparently working, both with text buffers and with PDFView buffers. Can someone check that this code isn't garbage? I have no idea what I'm doing with elisp. :)

(telephone-line-defsegment* telephone-line-airline-position-segment (&optional lines columns)
  "Position segment imitating vim-airline's appearance. Optional args set padding on lines/columns."
  (let* ((l (number-to-string (if lines lines 4)))
         (c (number-to-string (if columns columns 3))))
    (if (eq major-mode 'paradox-menu-mode)
        (telephone-line-raw mode-line-front-space t)
      (if (string-match-p "L[0-9]+\s*$" (telephone-line-raw mode-line-position t))
	  `((-3 "%p") ,(concat " %" l "l"
			       ":%" c (if (bound-and-true-p column-number-indicator-zero-based) "c" "C")))
	mode-line-position)
)))

[leonardo-cavegliacurtil]

I got it to work with this regex /^[\w\s%]*L\d+\s*$/, which should match all standard positions in text buffers.

(telephone-line-defsegment* telephone-line-airline-position-segment (&optional lines columns)
  "Position segment imitating vim-airline's appearance. Optional args set padding on lines/columns."
  (let* ((l (number-to-string (if lines lines 4)))
         (c (number-to-string (if columns columns 3))))
    (if (eq major-mode 'paradox-menu-mode)
        (telephone-line-raw mode-line-front-space t)
      (if (string-match-p "^[[:alnum:]\s%]*L[0-9]+\s*$" (telephone-line-raw mode-line-position t))
	  `((-3 "%p") ,(concat " %" l "l"
			       ":%" c (if (bound-and-true-p column-number-indicator-zero-based) "c" "C")))
	mode-line-position)
)))

I'm not sure if it's a clean solution to match against (telephone-line-raw mode-line-position t). But in case the code is sound, I have a branch ready for a pull.

[dbordak]

This sounds correct, but I don't remember best practices on elisp regex...

[leonardo-cavegliacurtil]

Any updates on this? What is exactly that is missing?

[dbordak]

I think selecting on major mode would be a cleaner solution, even if it means that it needs to be expanded in the future. I can't tell conclusively all the text formats that various modes use (i.e. it's possible that some of them would match the given regex, somehow), but on the other hand, modes which overwrite this value are probably going to be relatively few (likely only the ones which aren't rendering text, like pdfview).

EDIT: also another argument for, there's already a major-mode check in this function. Also, it'd be immediately obvious to anyone looking at the function why this check is necessary, vs the regex probably needing a line comment at least.

Something like this?

(telephone-line-defsegment* telephone-line-airline-position-segment (&optional lines columns)
  "Position segment imitating vim-airline's appearance. Optional args set padding on lines/columns."
  (let* ((l (number-to-string (if lines lines 4)))
         (c (number-to-string (if columns columns 3))))
    (cond
      ((eq major-mode 'paradox-menu-mode) (telephone-line-raw mode-line-front-space t))
      ((eq major-mode 'pdf-view-mode) (telephone-line-raw mode-line-position t))
      (t `((-3 "%p") ,(concat " %" l "l"
                              ":%" c (if (bound-and-true-p column-number-indicator-zero-based) "c" "C")))))))

[leonardo-cavegliacurtil]

That works just fine, and yeah, the regex does seem like a bit of a workaround.
The reason why I originally said that a major mode check seems "less clean" is that it's a hardcoded switch, that will inevitably end up growing.
I also found another mode that requires an exception, scad-preview-mode, from the scad-mode package.
I tested your code, added the following line, and it works as expected.

      ((eq major-mode 'scad-preview-mode) (telephone-line-raw mode-line-position t))

---

I'd propose to expose a customizable list of major modes, defaulting to those we know so far.
While not necessary, it would be a more permanent solution; although, at that point, it would also call for many other customizable lists and strings throughout the package.
I don't want to perturb the project too much, so it's up to you on whether that's worth considering.

That aside, the issue is to be considered solved, when this is patch is on MELPA.