Properly handle final path errors

[orhun]

Fixes #91

The thing is, when the question mark is operator is used, the error is returned to main:

tere/src/main.rs

Lines 69 to 72 in 8a97cd4

	let final_path = TereAppState::init(settings, &warnings)
	.and_then(|state| TereTui::init(state, &mut stderr))
	// actually run the app and return the final path
	.and_then(|mut ui| ui.main_event_loop())?;

Thus the next error handling part is never executed:

tere/src/main.rs

Lines 80 to 96 in 8a97cd4

	let (final_path, warnings) = match res {
	Err(err) => {
	match err {
	// Print pretty error message if the error was in arg parsing
	TereError::Clap(e) => e.exit(),
	TereError::ExitWithoutCd(msg) | TereError::FirstRunPromptCancelled(msg) => {
	eprintln!("{msg}");
	std::process::exit(1);
	}
	// exit in case of any other error
	e => return Err(e),
	}
	}
	Ok(path) => path,
	};

This PR removes the question mark operator and uses the good old match arms to return the error to the actual scope.

[mgunyho]

Ah, good catch! I think I somehow thought that the question mark would only go up to the enclosing block, but it goes all the way up because the block is not a function.

This same can be achieved with another and_then: instead if final_path = ..., we can just have

                TereAppState::init(settings, &warnings)
                    .and_then(|state| TereTui::init(state, &mut stderr))
                    // actually run the app and return the final path
                    .and_then(|mut ui| ui.main_event_loop())
                    .and_then(|final_path| Ok((final_path, warnings)))

(note no semicolon at the end)

I think we should also check the first question mark (at the end of chain producing (settings, warnings)), whether it should bubble all the way up, I'll think about it a bit later today or tomorrow.

Edit: oh, clippy tells me that it can be just

.map(|final_path| (final_path, warnings))

[mgunyho]

Yeah for example if I delete (or rename) the history json file so I get the first run prompt, cancelling it prints

Error: FirstRunPromptCancelled("Cancelled.")

even though it should be just Cancelled.. So I don't think we want the first question mark to bubble up either.

The full execution could be just chained together into a big sequence:

                stderr
                    .flush()
                    .map_err(TereError::from)
                    .and_then(|_| TereSettings::parse_cli_args(&cli_args))
                    .and_then(|(settings, warnings)| {
                        check_first_run_with_prompt(&settings, &mut stderr)?;
                        Ok((settings, warnings))
                    })
                    .and_then(|(settings, warnings)| {
                        TereAppState::init(settings, &warnings)
                            .and_then(|state| TereTui::init(state, &mut stderr))
                            // actually run the app and return the final path
                            .and_then(|mut ui| ui.main_event_loop())
                            .map(|final_path| (final_path, warnings))
                    })

I'm not sure why I didn't do it this way originally, maybe this is a bit harder to follow.

[orhun]

Sorry for the late reply.

That makes a lot of sense. Made the change in b9e06ef

[mgunyho]

No worries, thanks! I'm currently down a rabbit hole of pseudo-terminals and terminal escape code handling, trying to write an integration test that would check the stdout/stderr.

[orhun]

Definitely sounds interesting, let me know if you need any changes in the PR anytime soon!

[mgunyho]

Okay I've added (failing) tests for the output of the whole app in 70d2686. I'll rebase this on top of it to check that the tests pass and then merge.

[mgunyho]

Thanks!