Add option to fill window with stories

[nickdrozd]

If hackernews-items-per-page is nil, the number of stories is three
less than the window height. It isn't the full window height because
that includes the status bar and the minibuffer, and it's annoying to
have to scroll down to see the last few stories.

[basil-conto]

Add option to fill window with stories

Nice idea.

If hackernews-items-per-page is nil, the number of stories is three
less than the window height. It isn't the full window height because
that includes the status bar and the minibuffer, and it's annoying to
have to scroll down to see the last few stories.

I assume by "status bar" you mean the mode line and by "minibuffer" you mean its window, i.e. the echo area. Either way, I see various issues with trying to correctly fill the entire window with items:

1. In hackernews--load-stories, the selected window is not necessarily the same window as the one that will display the *hackernews ...* buffer. In general, it's not possible to know a priori how big the window will be; Emacs window management is much more complicated than that. In my configuration, for example, I almost never create new windows, but rather pop up frames because I use a tiling window manager.

2. In general, you should avoid hard-coding arbitrary calculations like (- (window-height) 3) and let Emacs tell you how many lines a window contains, because you have to account for the header and mode lines, as well as horizontal scroll bars, non-default faces, etc. A closer function would be window-body-height or, even better, window-text-height. See (elisp) Window Sizes for more on this.

3. A problem specific to hackernews is that we currently provide the user options hackernews-item-format, hackernews-score-format, hackernews-title-format, and hackernews-comments-format (and in the future may move to or additionally support a different UI such as tabulated-list-mode). These user options mean counting the number of items that fit in a window is not straightforward, and may need to be implemented as a heuristic.

Perhaps a better way would be to allow hackernews-items-per-page to be a function. In this case, you could set it to window-text-height and others could set it to a custom function which takes their setup into account. I will need to sleep on this. In the meantime, the best way to scratch your itch is probably to use advice, e.g.:

(define-advice hackernews (:around (fn &rest args) my-dynamic-height)
  "Adapt `hackernews-items-per-page' to current window height."
  (let ((hackernews-items-per-page (window-text-height)))
    (apply fn args)))

[basil-conto]

Just some suggestions for future reference.

[basil-conto]

Changes to user options should be accompanied by updates to their :type and :version/:package-version tags.

[basil-conto]

A more Elisp-y way of writing this would have been

(cond (n (prefix-numeric-value n))
      (hackernews-items-per-page)
      (t (window-text-height)))

[basil-conto]

(Just thinking aloud.)

Perhaps a better way would be to allow hackernews-items-per-page to be a function.

When considering this avenue, we should think about what, if anything, to pass to this function, whether via explicit argument or its environment, e.g. the current buffer, so that the function can make a more educated or flexible calculation. It may, for example, like to know the type of the feed being retrieved.

[nickdrozd]

It's always educational to open a PR here, even if they never get accepted! That advice does what I want, although I changed it to (1- (window-text-height)) because I like to be able to jump to the bottom of the buffer without any scrolling.

There might still be something here worth pursuing, so I'll leave this open (I've updated it to incorporate your suggestions). Consider it a prototyped feature request. But as I said, the advice works, so feel free to close it.

[basil-conto]

Perhaps a better way would be to allow hackernews-items-per-page to be a function.

When considering this avenue, we should think about what, if anything, to pass to this function, whether via explicit argument or its environment, e.g. the current buffer, so that the function can make a more educated or flexible calculation. It may, for example, like to know the type of the feed being retrieved.

I'll wait to design this option after #40 is fixed, as the latter will entail a large overhaul of the hackernews core.