cursive icon indicating copy to clipboard operation
cursive copied to clipboard

[BUG] Calling find_name() in the wrong order makes things unfindable

Open c-jiph opened this issue 10 months ago • 2 comments

Describe the bug The repro below panics with "tv" being None when unwrapped. If the two find_name() calls are swapped there is no problem.

I don't think this is mentioned as a something to be avoided anywhere. I am still fairly new to Rust so my apologies if there is something idiomatic I'm missing here.

To Reproduce

use cursive::{
    view::Nameable,
    views::{Dialog, TextView},
};

fn main() {
    let mut siv = cursive::default();

    siv.add_layer(Dialog::around(TextView::new("tv").with_name("tv")).with_name("d"));

    let _a = siv.find_name::<Dialog>("d").unwrap();
    let _b = siv.find_name::<TextView>("tv").unwrap();
}

Expected behavior Order of calls to find_name() makes no difference. If the problem was just I should drop the result of find_name() before making another call, that would be fine with me provided it were documented/enforced.

Environment

  • Operating system used: macOS 13.5.1
  • Backend used: ncurses
  • Current locale: en_US.UTF-8
  • Cursive version: 75bc082350e7d5c3d006040d3392f524d5170f55

c-jiph avatar Sep 07 '23 03:09 c-jiph

Hi, and thanks for the report!

If the problem was just I should drop the result of find_name() before making another call, that would be fine with me provided it were documented/enforced.

We can and should indeed better document it!

The result from find_name "locks" the view you found, and prevents finding child views while this lock is kept. (Why do we lock the view? So we can implement DerefMut, which lets users directly call methods on the result.)

This is why it's usually preferred to use call_on_name (it's harder to mix up locks this way).

I'm not sure we could enforce it though: it's valid to use find_name on disjoint views, it's only an issue when one is nested another the other one.

gyscos avatar Sep 10 '23 17:09 gyscos

Thanks for getting back to me.

I'm not sure we could enforce it though: it's valid to use find_name on disjoint views, it's only an issue when one is nested another the other one.

I was imagining find_name would return some kind of guard which would mark the view as "locked". Then whenever find_name is called it would search from child to root checking to make sure everything is "unlocked", panicing if not. Once the guard is dropped the lock would be released.

Having said that, I hadn't considered call_on_name and TBH wasn't really sure why this would be used until now. Does call_on_name have the same issue if it's re-called in a nested context? If not it sounds like this is also valid way to enforce things and perhaps the find_name doc can just recommend call_on_name.

Either way some mention in the docs would be most helpful.

c-jiph avatar Sep 10 '23 21:09 c-jiph