- Fix an issue that caused UTF-8 to be used when a file was closed and

R Aadarsh created

re-opened, while retaining the text.

- Fix an issue that prevented `InvalidBufferView` from being shown when
an incorrect encoding was chosen from the status bar.

- Centre the error message in `InvalidBufferView`.

Change summary

Cargo.lock                              |   1 
crates/encodings/src/lib.rs             |  16 ++-
crates/encodings_ui/Cargo.toml          |   1 
crates/encodings_ui/src/lib.rs          |  25 ++--
crates/encodings_ui/src/selectors.rs    | 130 +++++++++++++++++++-------
crates/language/src/buffer.rs           |   5 
crates/project/src/buffer_store.rs      |  12 ++
crates/project/src/invalid_item_view.rs |  19 ++-
crates/project/src/project.rs           |  11 --
crates/workspace/src/workspace.rs       |  10 +
10 files changed, 149 insertions(+), 81 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -5535,6 +5535,7 @@ dependencies = [
 name = "encodings_ui"
 version = "0.1.0"
 dependencies = [
+ "anyhow",
  "editor",
  "encoding_rs",
  "fs",

crates/encodings/src/lib.rs 🔗

@@ -136,23 +136,27 @@ pub async fn from_utf8(input: String, target: Encoding) -> anyhow::Result<Vec<u8
 }
 
 pub struct EncodingOptions {
-    pub encoding: Arc<Mutex<Encoding>>,
+    pub encoding: Arc<Encoding>,
     pub force: AtomicBool,
     pub detect_utf16: AtomicBool,
 }
 
 impl EncodingOptions {
-    pub fn reset(&mut self) {
-        self.encoding.lock().unwrap().reset();
-        *self.force.get_mut() = false;
-        *self.detect_utf16.get_mut() = true;
+    pub fn reset(&self) {
+        self.encoding.reset();
+
+        self.force
+            .store(false, std::sync::atomic::Ordering::Release);
+
+        self.detect_utf16
+            .store(true, std::sync::atomic::Ordering::Release);
     }
 }
 
 impl Default for EncodingOptions {
     fn default() -> Self {
         EncodingOptions {
-            encoding: Arc::new(Mutex::new(Encoding::default())),
+            encoding: Arc::new(Encoding::default()),
             force: AtomicBool::new(false),
             detect_utf16: AtomicBool::new(true),
         }

crates/encodings_ui/Cargo.toml 🔗

@@ -5,6 +5,7 @@ publish.workspace = true
 edition.workspace = true
 
 [dependencies]
+anyhow.workspace = true
 editor.workspace = true
 encoding_rs.workspace = true
 fs.workspace = true

crates/encodings_ui/src/lib.rs 🔗

@@ -74,17 +74,20 @@ impl Render for EncodingIndicator {
 
                             let weak_workspace = workspace.weak_handle();
 
-                            workspace.toggle_modal(window, cx, |window, cx| {
-                                let selector = EncodingSelector::new(
-                                    window,
-                                    cx,
-                                    Action::Save,
-                                    Some(buffer.downgrade()),
-                                    weak_workspace,
-                                    None,
-                                );
-                                selector
-                            })
+                            if let Some(path) = buffer.read(cx).file() {
+                                let path = path.clone().path().to_path_buf();
+                                workspace.toggle_modal(window, cx, |window, cx| {
+                                    let selector = EncodingSelector::new(
+                                        window,
+                                        cx,
+                                        Action::Save,
+                                        Some(buffer.downgrade()),
+                                        weak_workspace,
+                                        Some(path),
+                                    );
+                                    selector
+                                });
+                            }
                         }
                     })
                 }

crates/encodings_ui/src/selectors.rs 🔗

@@ -117,19 +117,23 @@ pub mod save_or_reopen {
 
                     let weak_workspace = workspace.read(cx).weak_handle();
 
-                    workspace.update(cx, |workspace, cx| {
-                        workspace.toggle_modal(window, cx, |window, cx| {
-                            let selector = EncodingSelector::new(
-                                window,
-                                cx,
-                                Action::Save,
-                                Some(buffer.downgrade()),
-                                weak_workspace,
-                                None,
-                            );
-                            selector
-                        })
-                    });
+                    if let Some(file) = buffer.read(cx).file() {
+                        let path = file.as_local()?.abs_path(cx);
+
+                        workspace.update(cx, |workspace, cx| {
+                            workspace.toggle_modal(window, cx, |window, cx| {
+                                let selector = EncodingSelector::new(
+                                    window,
+                                    cx,
+                                    Action::Save,
+                                    Some(buffer.downgrade()),
+                                    weak_workspace,
+                                    Some(path),
+                                );
+                                selector
+                            })
+                        });
+                    }
                 }
             } else if self.current_selection == 1 {
                 if let Some(workspace) = self.workspace.upgrade() {
@@ -142,19 +146,23 @@ pub mod save_or_reopen {
 
                     let weak_workspace = workspace.read(cx).weak_handle();
 
-                    workspace.update(cx, |workspace, cx| {
-                        workspace.toggle_modal(window, cx, |window, cx| {
-                            let selector = EncodingSelector::new(
-                                window,
-                                cx,
-                                Action::Reopen,
-                                Some(buffer.downgrade()),
-                                weak_workspace,
-                                None,
-                            );
-                            selector
+                    if let Some(file) = buffer.read(cx).file() {
+                        let path = file.as_local()?.abs_path(cx);
+
+                        workspace.update(cx, |workspace, cx| {
+                            workspace.toggle_modal(window, cx, |window, cx| {
+                                let selector = EncodingSelector::new(
+                                    window,
+                                    cx,
+                                    Action::Reopen,
+                                    Some(buffer.downgrade()),
+                                    weak_workspace,
+                                    Some(path),
+                                );
+                                selector
+                            });
                         });
-                    });
+                    }
                 }
             }
 
@@ -290,7 +298,7 @@ pub mod encoding {
         Context, HighlightedLabel, ListItem, ListItemSpacing, ParentElement, Render, Styled,
         Window, rems, v_flex,
     };
-    use util::{ResultExt, TryFutureExt};
+    use util::ResultExt;
     use workspace::{CloseActiveItem, ModalView, OpenOptions, Workspace};
 
     use crate::encoding_from_name;
@@ -438,35 +446,81 @@ pub mod encoding {
                 .upgrade()
                 .unwrap();
 
+            let weak_workspace = workspace.read(cx).weak_handle();
+
+            let current_selection = self.matches[self.current_selection].string.clone();
+
             if let Some(buffer) = &self.buffer
-                && let Some(buffer_entity) = buffer.upgrade()
+                && let Some(buffer) = buffer.upgrade()
             {
-                let buffer = buffer_entity.read(cx);
-
+                let path = self
+                    .selector
+                    .upgrade()
+                    .unwrap()
+                    .read(cx)
+                    .path
+                    .clone()
+                    .unwrap();
+
+                let reload = buffer.update(cx, |buffer, cx| buffer.reload(cx));
                 // Since the encoding will be accessed in `reload`,
                 // the lock must be released before calling `reload`.
                 // By limiting the scope, we ensure that it is released
+
                 {
+                    let buffer = buffer.read(cx);
+
                     let buffer_encoding = buffer.encoding.clone();
-                    buffer_encoding.set(encoding_from_name(
-                        self.matches[self.current_selection].string.as_str(),
-                    ));
+                    buffer_encoding.set(encoding_from_name(&current_selection.clone()));
                 }
 
                 self.dismissed(window, cx);
 
                 if self.action == Action::Reopen {
-                    buffer_entity.update(cx, |buffer, cx| {
-                        let rec = buffer.reload(cx);
-                        cx.spawn(async move |_, _| rec.await).detach()
+                    buffer.update(cx, |_, cx| {
+                        cx.spawn_in(window, async move |_, cx| {
+                            if let Err(_) | Ok(None) = reload.await {
+                                let workspace = weak_workspace.upgrade().unwrap();
+
+                                workspace
+                                    .update_in(cx, |workspace, window, cx| {
+                                        workspace
+                                            .encoding_options
+                                            .encoding
+                                            .lock()
+                                            .unwrap()
+                                            .set(encoding_from_name(&current_selection));
+
+                                        *workspace.encoding_options.force.get_mut() = false;
+
+                                        *workspace.encoding_options.detect_utf16.get_mut() = true;
+
+                                        workspace
+                                            .active_pane()
+                                            .update(cx, |pane, cx| {
+                                                pane.close_active_item(
+                                                    &CloseActiveItem::default(),
+                                                    window,
+                                                    cx,
+                                                )
+                                            })
+                                            .detach();
+
+                                        workspace
+                                            .open_abs_path(path, OpenOptions::default(), window, cx)
+                                            .detach()
+                                    })
+                                    .log_err();
+                            }
+                        })
+                        .detach()
                     });
                 } else if self.action == Action::Save {
-                    let task = workspace.update(cx, |workspace, cx| {
+                    workspace.update(cx, |workspace, cx| {
                         workspace
                             .save_active_item(workspace::SaveIntent::Save, window, cx)
-                            .log_err()
+                            .detach();
                     });
-                    cx.spawn(async |_, _| task).detach();
                 }
             } else {
                 if let Some(path) = self.selector.upgrade().unwrap().read(cx).path.clone() {

crates/language/src/buffer.rs 🔗

@@ -1369,7 +1369,7 @@ impl Buffer {
     /// Reloads the contents of the buffer from disk.
     pub fn reload(&mut self, cx: &Context<Self>) -> oneshot::Receiver<Option<Transaction>> {
         let (tx, rx) = futures::channel::oneshot::channel();
-        let encoding = self.encoding.clone();
+        let encoding = (*self.encoding).clone();
 
         let buffer_encoding = self.encoding.clone();
 
@@ -1377,8 +1377,9 @@ impl Buffer {
         self.reload_task = Some(cx.spawn(async move |this, cx| {
             let Some((new_mtime, new_text)) = this.update(cx, |this, cx| {
                 let file = this.file.as_ref()?.as_local()?;
+
                 Some((file.disk_state().mtime(), {
-                    file.load(cx, (*encoding).clone(), false, true, Some(buffer_encoding))
+                    file.load(cx, encoding, false, true, Some(buffer_encoding))
                 }))
             })?
             else {

crates/project/src/buffer_store.rs 🔗

@@ -633,6 +633,8 @@ impl LocalBufferStore {
         detect_utf16: bool,
         cx: &mut Context<BufferStore>,
     ) -> Task<Result<Entity<Buffer>>> {
+        println!("{:?}", encoding);
+
         let load_buffer = worktree.update(cx, |worktree, cx| {
             let reservation = cx.reserve_entity();
             let buffer_id = BufferId::from(reservation.entity_id().as_non_zero_u64());
@@ -681,8 +683,8 @@ impl LocalBufferStore {
                             entry_id: None,
                             is_local: true,
                             is_private: false,
-                            encoding: Some(Arc::new(if let Some(encoding) = encoding {
-                                encoding
+                            encoding: Some(Arc::new(if let Some(encoding) = &encoding {
+                                encoding.clone()
                             } else {
                                 Encoding::default()
                             })),
@@ -713,6 +715,12 @@ impl LocalBufferStore {
                 anyhow::Ok(())
             })??;
 
+            buffer.update(cx, |buffer, _| {
+                buffer
+                    .encoding
+                    .set(encoding.unwrap_or(Encoding::default()).get())
+            })?;
+
             Ok(buffer)
         })
     }

crates/project/src/invalid_item_view.rs 🔗

@@ -2,9 +2,9 @@ use std::{path::Path, sync::Arc};
 
 use gpui::{EventEmitter, FocusHandle, Focusable};
 use ui::{
-    h_flex, v_flex, App, Button, ButtonCommon, ButtonStyle, Clickable, Context, FluentBuilder,
-    InteractiveElement, KeyBinding, Label, LabelCommon, LabelSize, ParentElement, Render,
-    SharedString, Styled as _, TintColor, Window,
+    App, Button, ButtonCommon, ButtonStyle, Clickable, Context, FluentBuilder, InteractiveElement,
+    KeyBinding, Label, LabelCommon, LabelSize, ParentElement, Render, SharedString, Styled as _,
+    TintColor, Window, div, h_flex, v_flex,
 };
 use zed_actions::workspace::OpenWithSystem;
 
@@ -89,15 +89,18 @@ impl Render for InvalidItemView {
             .overflow_hidden()
             .key_context("InvalidBuffer")
             .child(
-                h_flex().size_full().justify_center().child(
+                h_flex().size_full().justify_center().items_center().child(
                     v_flex()
-                        .justify_center()
                         .gap_2()
+                        .max_w_96()
                         .child(h_flex().justify_center().child("Could not open file"))
                         .child(
-                            h_flex()
-                                .justify_center()
-                                .child(Label::new(self.error.clone()).size(LabelSize::Small)),
+                            h_flex().justify_center().child(
+                                div()
+                                    .whitespace_normal()
+                                    .text_center()
+                                    .child(Label::new(self.error.clone()).size(LabelSize::Small)),
+                            ),
                         )
                         .when(self.is_local, |contents| {
                             contents

crates/project/src/project.rs 🔗

@@ -109,7 +109,6 @@ use settings::{InvalidSettingsError, Settings, SettingsLocation, SettingsStore};
 use smol::channel::Receiver;
 use snippet::Snippet;
 use snippet_provider::SnippetProvider;
-use std::ops::Deref;
 use std::{
     borrow::Cow,
     collections::BTreeMap,
@@ -2723,15 +2722,7 @@ impl Project {
         self.buffer_store.update(cx, |buffer_store, cx| {
             buffer_store.open_buffer(
                 path.into(),
-                Some(
-                    self.encoding_options
-                        .encoding
-                        .lock()
-                        .as_ref()
-                        .unwrap()
-                        .deref()
-                        .clone(),
-                ),
+                Some((*self.encoding_options.encoding).clone()),
                 *self.encoding_options.force.get_mut(),
                 *self.encoding_options.detect_utf16.get_mut(),
                 cx,

crates/workspace/src/workspace.rs 🔗

@@ -651,7 +651,7 @@ impl ProjectItemRegistry {
                 let project_path = project_path.clone();
                 let encoding = encoding.unwrap_or_default();
 
-                project.update(cx, |project, _| {project.encoding_options.encoding.lock().unwrap().set(encoding.get())});
+                project.update(cx, |project, _| project.encoding_options.encoding.set(encoding.get()));
 
                 let is_file = project
                     .read(cx)
@@ -1945,6 +1945,8 @@ impl Workspace {
         window: &mut Window,
         cx: &mut Context<Workspace>,
     ) -> Task<Result<()>> {
+        // This is done so that we would get an error when we try to open the file with wrong encoding,
+        // and not silently use the previously set encoding.
         self.encoding_options.reset();
 
         let to_load = if let Some(pane) = pane.upgrade() {
@@ -3582,8 +3584,8 @@ impl Workspace {
             project.encoding_options.force.store(
                 self.encoding_options
                     .force
-                    .load(std::sync::atomic::Ordering::Relaxed),
-                std::sync::atomic::Ordering::Relaxed,
+                    .load(std::sync::atomic::Ordering::Acquire),
+                std::sync::atomic::Ordering::Release,
             );
         });
 
@@ -3591,7 +3593,7 @@ impl Workspace {
         registry.open_path(
             project,
             &path,
-            Some(self.encoding_options.encoding.lock().unwrap().clone()),
+            Some((*self.encoding_options.encoding).clone()),
             window,
             cx,
         )