Skip to content

display: implement DisplayLevel for genbooks (fix issue #528)#1292

Merged
karlkleinpaste merged 2 commits intocrosswire:masterfrom
LAfricain:genBook
Mar 15, 2026
Merged

display: implement DisplayLevel for genbooks (fix issue #528)#1292
karlkleinpaste merged 2 commits intocrosswire:masterfrom
LAfricain:genBook

Conversation

@LAfricain
Copy link
Contributor

Xiphos was always behaving as if DisplayLevel=1 for genbooks, regardless
of the value set in the module configuration. This meant that clicking on
a node in the tree would only display the content of that single node,
with no way to aggregate child content into a single scrollable view.

This fix reads the DisplayLevel config entry from the module and, when
it is greater than 1, recursively renders the current node and its
descendants up to the specified depth, concatenating their content with
a horizontal rule separator between each node.

Changes:

  • display.cc: add #include <treekeyidx.h>
  • display.cc: add static helper _render_display_level() which traverses
    the TreeKeyIdx tree by offset, rendering each node's content into a
    SWBuf, up to max_level depth
  • display.cc: in GTKEntryDisp::display(), read DisplayLevel from module
    config and call _render_display_level() when value > 1, falling back
    to the original single-node rendering when DisplayLevel is absent or 1

Module maintainers who want to benefit from this feature need to add
DisplayLevel=N to their module's .conf file, where N reflects the depth
of the tree they want rendered on a single click.

Fixes #528

GLOBAL_OPS *ops, BackEnd *be)
{
int x = 0;
gchar heading[8];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of increasing the size of the buffer here, would it be better to change the uses of sprintf to snprintf (which takes a maximum number of characters to print)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like that:
sprintf(heading, "%d", x);
by :
csnprintf(heading, sizeof(heading), "%d", x);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be snprintf(heading, sizeof(heading), "%d", x); (without the c at the start) but yes that's the right idea. I think it needs done on lines 735 and 750.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the waring remains with snprintf:

/home/cyrille/Documents/github/xiphos/src/main/display.cc: In member function ‘virtual char GTKPrintChapDisp::display(sword::SWModule&)’:
/home/cyrille/Documents/github/xiphos/src/main/display.cc:2226:61: warning: ‘%d’ directive output may be truncated writing between 1 and 10 bytes into a region of size 8 [-Wformat-truncation=]
 2226 |                         snprintf(heading, sizeof(heading), "%d", x);
      |                                                             ^~
/home/cyrille/Documents/github/xiphos/src/main/display.cc:2226:60: note: directive argument in the range [1, 2147483647]
 2226 |                         snprintf(heading, sizeof(heading), "%d", x);
      |                                                            ^~~~
/home/cyrille/Documents/github/xiphos/src/main/display.cc:2226:33: note: ‘snprintf’ output between 2 and 11 bytes into a destination of size 8
 2226 |                         snprintf(heading, sizeof(heading), "%d", x);
      |                         ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason I cannot see those warnings on my build. What command produces that output for you?

@karlkleinpaste
Copy link
Contributor

FYI I'm avoiding merging this one until this question is resolved.

@LAfricain
Copy link
Contributor Author

Actually, you can go ahead and merge it. I have plenty of build errors that I’ve never reported. I looked into this one because it was happening at the end of the build and it was bothering me—I wanted it to go faster. I’ve created another branch where I’ll address all the other errors I encounter.

@karlkleinpaste
Copy link
Contributor

Well... OK.

@karlkleinpaste karlkleinpaste merged commit c7c07e5 into crosswire:master Mar 15, 2026
8 checks passed
@LAfricain
Copy link
Contributor Author

Since Claude is taking a break, I took the opportunity to clean up my code base in preparation for our next adventures with Claude, and now I don't have any compilation errors at all! So that's great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Xiphos ignores DisplayLevel option in genbooks

3 participants