Use BString as editor return value to not enforce encodings.

This will make it harder to call from legacy code, but will make it
clear for modern code that we don't know the encoding and shouldn't
lighthartedly butcher it by converting to unicode lossily.

Note that all callers now state explicitly that they handle the message
lossily, and we should fix this once the code is modernised.

While at it, also avoid rewriting line endings in favor of reusing
the ones that the user specified.

Author: Byron

crates/but/src/command/legacy/commit.rs

@@ -497,8 +497,9 @@ fn get_commit_message_from_editor(
     template.push_str("#\n");
 
     // Read the result from the editor and strip comments
-    let message = tui::get_text::from_editor_no_comments("commit_msg", &template)?;
-    Ok(message)
+    let lossy_message =
+        tui::get_text::from_editor_no_comments("commit_msg", &template)?.to_string();
+    Ok(lossy_message)
 }
 
 fn get_status_char(path: &BString, changes: &[TreeChange]) -> &'static str {

crates/but/src/command/legacy/describe.rs

@@ -236,13 +236,14 @@ fn get_commit_message_from_editor(
     template.push_str("#\n");
 
     // Read the result and strip comments
-    let message = tui::get_text::from_editor_no_comments("commit_msg", &template)?;
+    let lossy_message =
+        tui::get_text::from_editor_no_comments("commit_msg", &template)?.to_string();
 
-    if message.is_empty() {
+    if lossy_message.is_empty() {
         bail!("Aborting due to empty commit message");
     }
 
-    Ok(message)
+    Ok(lossy_message)
 }
 
 fn get_branch_name_from_editor(current_name: &str) -> Result<String> {
@@ -255,13 +256,14 @@ fn get_branch_name_from_editor(current_name: &str) -> Result<String> {
     template.push_str("# with '#' will be ignored, and an empty name aborts the operation.\n");
     template.push_str("#\n");
 
-    let branch_name = tui::get_text::from_editor_no_comments("branch_name", &template)?;
+    let branch_name_lossy =
+        tui::get_text::from_editor_no_comments("branch_name", &template)?.to_string();
 
-    if branch_name.is_empty() {
+    if branch_name_lossy.is_empty() {
         bail!("Aborting due to empty branch name");
     }
 
-    Ok(branch_name)
+    Ok(branch_name_lossy)
 }
 
 #[cfg(test)]

crates/but/src/command/legacy/forge/review.rs

@@ -564,8 +564,8 @@ fn get_review_body_from_editor(
     template.push_str("# with '#' will be ignored, and an empty body is allowed.\n");
     template.push_str("#\n");
 
-    let body = get_text::from_editor_no_comments("review_body", &template)?;
-    Ok(body)
+    let lossy_body = get_text::from_editor_no_comments("review_body", &template)?.to_string();
+    Ok(lossy_body)
 }
 
 /// Extract the commit description (body) from the commit message, skipping the first line (title).
@@ -606,13 +606,13 @@ fn get_review_title_from_editor(
     template.push_str("# with '#' will be ignored, and an empty title aborts the operation.\n");
     template.push_str("#\n");
 
-    let title = get_text::from_editor_no_comments("review_title", &template)?;
+    let lossy_title = get_text::from_editor_no_comments("review_title", &template)?.to_string();
 
-    if title.is_empty() {
+    if lossy_title.is_empty() {
         anyhow::bail!("Aborting due to empty review title");
     }
 
-    Ok(title)
+    Ok(lossy_title)
 }
 
 /// Extract the commit title from the commit message (first line).

crates/but/src/tui/get_text.rs

@@ -1,31 +1,32 @@
 //! Various functions that involve launching the Git editor (i.e. `GIT_EDITOR`).
-use anyhow::Result;
-use bstr::ByteSlice;
+use anyhow::{Result, bail};
+use bstr::{BStr, BString, ByteSlice};
 use std::ffi::OsStr;
 
 /// Launches the user's preferred text editor to edit some `initial_text`,
 /// identified by a `filename_safe_intent` to help the user understand what's wanted of them.
 /// Note that this string must be valid in filenames.
 ///
-/// Returns the edited text, with comment lines (starting with `#`) removed.
-pub fn from_editor_no_comments(filename_safe_intent: &str, initial_text: &str) -> Result<String> {
+/// Returns the edited text (*without known encoding*), with comment lines (starting with `#`) removed.
+pub fn from_editor_no_comments(filename_safe_intent: &str, initial_text: &str) -> Result<BString> {
     let content = from_editor(filename_safe_intent, initial_text)?;
 
     // Strip comment lines (starting with '#')
-    let filtered_lines: Vec<&str> = content
-        .lines()
-        .filter(|line| !line.trim_start().starts_with('#'))
+    let filtered_lines: Vec<&BStr> = content
+        .lines_with_terminator()
+        .filter(|line| !line.trim_start().starts_with_str("#"))
+        .map(|line| line.as_bstr())
         .collect();
 
-    Ok(filtered_lines.join("\n").trim().to_string())
+    Ok(filtered_lines.into_iter().collect())
 }
 
 /// Launches the user's preferred text editor to edit some `initial_text`,
 /// identified by a `filename_safe_intent` to help the user understand what's wanted of them.
 /// Note that this string must be valid in filenames.
 ///
-/// Returns the edited text verbatim.
-pub fn from_editor(identifier: &str, initial_text: &str) -> Result<String> {
+/// Returns the edited text (*without known encoding*) verbatim.
+pub fn from_editor(identifier: &str, initial_text: &str) -> Result<BString> {
     let editor_cmd = get_editor_command()?;
 
     // Create a temporary file with the initial text
@@ -46,9 +47,9 @@ pub fn from_editor(identifier: &str, initial_text: &str) -> Result<String> {
         .wait()?;
 
     if !status.success() {
-        return Err(anyhow::anyhow!("Editor exited with non-zero status"));
+        bail!("Editor exited with non-zero status");
     }
-    Ok(std::fs::read_to_string(&tempfile)?)
+    Ok(std::fs::read(&tempfile)?.into())
 }
 
 /// Get the user's preferred editor command.