Added segments for window number and projectile.

[anandpiyer]

This PR adds two simple segments, one to show the window number while using winum-mode and the other to show a clickable projectile project name.

[dbordak]

These should probably be guarded with eval-after-load... I don't completely remember the reasoning behind doing that over just checking if symbols are bound (faster load times?), but at this point it's probably better to keep it consistent.

[anandpiyer]

I think there are two issues with using eval-after-load: (1) it would stop telephone-line from making the window number segment generic in the future (there are other packages that have similar functionality, e.g., window-numbering), and (2) my understanding was that it interferes with byte-compilation.

I'm an elisp beginner, but I thought the use of eval-after-load is discouraged in general: https://www.gnu.org/software/emacs/manual/html_node/elisp/Hooks-for-Loading.html

If you still want to do it with eval-after-load, I have no problem in wrapping these in it. Let me know!

[dbordak]

It probably isn't necessary here since we're just exchanging one error message for another, neither of which are really helpful, but I don't get the impression that the page you sent discourages the use of eval-after-load; rather that you shouldn't be using it instead of checking for symbol bindings. You can't just do that for functions, obviously, but the solution listed on that page is to just require the package if you need a function... which isn't something you can do here (the entire thing seems to be written assuming no one ever has optional dependencies?)

Anyway, the reason for it was probably one of 3 things:

1. the error you get without these is much longer than the one you get with it
2. I did it to avoid cluttering up the namespace when you don't have a lot of optdepends?
3. If you load telephone-line before one of these optdepends, you'll get a bunch of garbage in your message buffer on startup.

But if it interferes with byte-compilation then yeah I guess I should remove it. Do you have any citation for that?

I don't think your point 1 is really important; if anyone wanted to genericize it in the future, they could just remove the guard. In fact, there's already one example of this; look at both of the position segments. (those also definitely don't need the guard since it's only checking symbols, I should at least go back and remove guards in other cases like that)

[anandpiyer]

The byte compilation reference is from SO: https://stackoverflow.com/questions/21880139/what-is-with-eval-after-load-in-emacs-lisp

[dbordak]

From that link, sounds like it's just eval-after-load that can cause problems, not with-eval-after-load. With that in mind, I think the best thing to do would be to update all the existing forms to with-eval-after-load (the package already depends upon Emacs 24.4 anyway). Thanks for that, I had no idea!

[anandpiyer]

I'm not sure if this would work for packages that are loaded after telephone line is loaded? Say, for example, I use nyan-mode, but only load it in some modes. Then I also need a conditional in setting telephone line lhs/rhs where I use telephone-line-nyan-segment isn't it?

[dbordak]

It happens when the package is loaded (require'd), not when it's enabled.

EDIT: Also in the case of nyan-mode, I don't think enabling nyan-mode has any bearing on whether the segment works or not (I don't use that segment so I'd need to check).

[anandpiyer]

It won't work if someone is using deferred loading, say for instance, using use-package. Here's a simple example where it breaks:

(use-package nyan-mode
  :ensure t :defer t)

(use-package telephone-line
  :ensure t
  :init

  (setq telephone-line-lhs
      '((nil    . (telephone-line-nyan-segment))
        (accent . (telephone-line-major-mode-segment))
        (evil   . (telephone-line-airline-position-segment))))

  (setq telephone-line-rhs
      '((nil    . (telephone-line-misc-info-segment))
        (accent . (telephone-line-major-mode-segment))
        (evil   . (telephone-line-airline-position-segment))))

  (telephone-line-mode 1))

[dbordak]

Well yeah, but that wouldn't work even without eval-after-load. The nyan segment calls a function from nyan-mode, if it's not loaded, it won't work.

EDIT: In fact, that example is using the :defer keyword incorrectly: https://github.com/jwiegley/use-package#notes-about-lazy-loading. You're only supposed to use :defer if you know the package will end up loaded before it's used -- which it isn't in either case, here.

[anandpiyer]

It won't work even if you remove the :defer keyword if the user only wants it for certain modes, for example:

(use-package nyan-mode
:ensure t
:mode "\\.py\\'")

(use-package telephone-line
  :ensure t
  :init

  (setq telephone-line-lhs
      '((nil    . (telephone-line-nyan-segment))
        (accent . (telephone-line-major-mode-segment))
        (evil   . (telephone-line-airline-position-segment))))

  (setq telephone-line-rhs
      '((nil    . (telephone-line-misc-info-segment))
        (accent . (telephone-line-major-mode-segment))
        (evil   . (telephone-line-airline-position-segment))))

  (telephone-line-mode 1))

Assuming that users will always write correct code is bad in my humble experience :-)

My point is that it will work in all cases if we use bound* check Instead of with-eval-after-load. The following works:

(use-package nyan-mode
:ensure t
:mode "\\.py\\'")

(use-package telephone-line
  :ensure t
  :init

  (telephone-line-defsegment my-telephone-line-nyan-segment ()
    (when (bound-and-true-p nyan-mode)
              (nyan-create)))

  (setq telephone-line-lhs
      '((nil    . (my-telephone-line-nyan-segment))
        (accent . (telephone-line-major-mode-segment))
        (evil   . (telephone-line-airline-position-segment))))

  (setq telephone-line-rhs
      '((nil    . (telephone-line-misc-info-segment))
        (accent . (telephone-line-major-mode-segment))
        (evil   . (telephone-line-airline-position-segment))))

  (telephone-line-mode 1))

[dbordak]

Alright, that's a fair point. Then I can have the default config include everything