"List visible monsters" and "List visible items" behavior

[fruviad]

This is about a trivial annoyance, but it occurs constantly.

You are playing a mage at depth, in an empty corridor next to a massive crypt. Type '[' to show the monster list and it informs you that you can see no monsters. Then, you absently type ']' twice to flip over to the item list, and the (empty) monster list disappears, having been replaced by the item list.

Now, cast Reveal Monsters. XP from your wildest dreams -- with promise of loot to follow -- is displayed on the other side of that stone wall. You view the monster list with '[' and scan the top of the list, satisfying yourself that it'll be easy.

Then, you type ']' to make the monster list go away like it did 10 seconds before. But that no longer works.

The UI has changed because the monster list now extends past the bottom of your window. You've been shunted to src/ui-output.c, line 195, which handles closing the display of text longer than your window:

   else if (ch.code== ESCAPE || ch.code == 'q')

Ten seconds ago, any key closed your monster list. Suddenly, for arbitrary reasons, you have to hit ESCAPE (or "q") to get out of the monster list.

The UI should be consistent for the user, but this isn't consistent.

Without having done any review of the implications of the change, I tried changing the src/ui-output.c line 195 code to this:

   else if (ch.code == ESCAPE || ch.code == 'q' || ch.code == ']' || ch.code == '[')

I recompiled, tested for all of 30 seconds, and COOL, it worked! Very impressive, given that I don't know what I'm doing.

Then I stepped back and tried to figure out the implications of this. I grepped through files and figured out that the textui_textblock_show() function is used in several of the ui-*.c files, so maybe isn't a good solution?

I've done a little more testing -- but nothing exhaustive -- and it seems as though this doesn't break anything, although I can see why might be a bad thing from a code management perspective to modify the function to suit two specific uses of it.

To my untrained eye, it looks like many of the instances where the function is called are dealing with short amounts of text that wouldn't trigger the above "else if" case. I padded the copyright text (one thing that's displayed using this function) and nothing bad seemed to occur with the function in that context; the altered code simply gave me 2 more keys that would close that text display.

Thoughts? Is this itch even worth scratching? Is this entirely the wrong way to go about it?

[Nick]

Looks good at first glance