The changes seem to achieve the correct results (looking at them, not yet tested), but the function looks already terribly fragile and weird (i.e., what if one of the expected answers was `CANCEL` already?). The odd code here was mapping `"Cancel"` label to `RESPONSE_CANCEL`, and back `RESPONSE_CANCEL` to whatever response was associated with the `"Cancel"` label. Meh. As you say, discarding the dialog emits `RESPONSE_DELETE_EVENT`, not `CANCEL`, so it was effectively useless. Also, knowing the dialog was canceled might be a valuable information to the caller, and it could be handled differently than `response_2` (and why choose `response_2` in the first place? it doesn't have any semantic to it).
Also, it would probably be more solid to `return ret != GTK_RESPONSE_APPLY` instead of `return ret == GTK_RESPONSE_NO` in `kb_find_duplicate()`, so it *allows* only if the response really is `APPLY` and not in any other possible case, no matter what is the response (another bug, future thing in the function, etc.). I fixed this as I suggest in d7750a44796b0ddc1c7a8968fd53aad91382d686. But thanks for the report, diagnostic and offering a fix! --- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/geany/geany/pull/714#issuecomment-219591953
