On Mon, Apr 6, 2026 at 12:22 PM Daniel Sahlberg <[email protected]> wrote:
> Hi, > > I took a look at svnbrowse today and it is coming together nicely! Of > course I had to try to break it as well... I suppose you are already aware > but it would make life easier when testing if we could sort this out. > Thanks for testing, nice! > I picked a random commit trying to explain. > > Den ons 1 apr. 2026 kl 22:53 skrev <[email protected]>: > > ... > > >> >> static svn_error_t * >> sub_main(int *code, int argc, char *argv[], apr_pool_t *pool) >> { >> > > ...sub_main initializes the screen... > > >> /* init the display */ >> initscr(); >> >> > Then later on when processing some keypresses, we call SVN_ERR() > > >> + else if (ch == KEY_BACKSPACE || ch == '-' || ch == 'u') >> + { >> + const char *new_url = svn_relpath_dirname(ctx.relpath, pool); >> + SVN_ERR(enter_path(&ctx, new_url, pool)); >> + } >> > > SVN_ERR return whenever there is an error. This leads to > > quit: > endwin(); > > never being executed, thus leaving the terminal in an unusable state. > (`reset` works fine to restore it). > > I initially thought about a global variable and moving endwin() to the > proper main(), but thinking twice it might be better to just move the > keypress processing loop to its own function. > > Thoughts? > > Yes, this problem exists. Moving keypress handling to a separate function does make sense but then it's not so obvious to make cancellation (by q/esc). We could throw an error, but the question is which one. Perhaps SVN_ERR_CANCELLED but it could be produced by libsvn as well. The issue with doing all of that in main is that curses methods could result in an error. We don't handle any of these yet, but technically should. $ man endwin [[[ ... • endwin returns ERR if • the terminal was not initialized, • it is called more than once without updating the screen, or • its call of reset_shell_mode(3X) returns ERR; and ... ]]] Which we would like to handle with svn_error in future. Also we can always replace SVN_ERR() use with manual checks to safely quit with an error. I think we should also set up handlers for abort (svn_error_set_malfunction_handler) to reset terminal state and signal handling (CTRL+C). By the way, did it crash by entering a file? Because I don't know yet what the behaviour should be. Obviously we don't want to crash. Also we receive node type when listing a directory so we could almost certainly tell what node type it will be. However, we can't because it can change since we listed it to the moment we entered this node. I suggest we first assume that it didn't change, and in this rare occasion, if that operation created an error, our program can change its mind and try to fetch it as a different type. At the moment we refetch it, it may change its type back. However, the same problem would happen if the node was removed entirely. -- Timofei Zhakov
