83b6042 · helix · git.iko.soy

git.iko.soy/helix

on master help sign in

fix(write): do not set new path on document until write succeeds

83b6042Skyler Hawthorne5 years ago

helix-term/src/application.rs+18−8

compare with master

unifiedsplit

@@ -3,6 +3,7 @@ use futures_util::Stream;

3use helix_core::{

4 config::{default_syntax_loader, user_syntax_loader},

5 diagnostic::{DiagnosticTag, NumberOrString},

6+ path::get_relative_path,

6 pos_at_coords, syntax, Selection,

7 pos_at_coords, syntax, Selection,

7};

8};

8use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap};

9use helix_lsp::{lsp, util::lsp_pos_to_pos, LspProgressMap};

@@ -489,17 +490,26 @@ impl Application {

489 );

490 );

490

491

491 doc.set_last_saved_revision(doc_save_event.revision);

492 doc.set_last_saved_revision(doc_save_event.revision);

493+

492 let lines = doc.text().len_lines();

494 let lines = doc.text().len_lines();

493 let bytes = doc.text().len_bytes();

495 let bytes = doc.text().len_bytes();

494

496

495− let path_str = doc

497+ if let Err(err) = doc.set_path(Some(&doc_save_event.path)) {

496− .path()

498+ log::error!(

497− .expect("document written without path")

499+ "error setting path for doc '{:?}': {}",

498− .to_string_lossy()

500+ doc.path(),

499− .into_owned();

501+ err.to_string(),

500−

502+ );

503+ self.editor.set_error(err.to_string());

504+ } else {

505+ // TODO: fix being overwritten by lsp

501− self.editor

506+ self.editor.set_status(format!(

502− .set_status(format!("'{}' written, {}L {}B", path_str, lines, bytes));

507+ "'{}' written, {}L {}B",

508+ get_relative_path(&doc_save_event.path).to_string_lossy(),

509+ lines,

510+ bytes

511+ ));

512+ }

503 }

513 }

504

514

505 pub fn handle_terminal_events(&mut self, event: Result<CrosstermEvent, crossterm::ErrorKind>) {

515 pub fn handle_terminal_events(&mut self, event: Result<CrosstermEvent, crossterm::ErrorKind>) {

helix-term/src/commands/typed.rs+2−7

@@ -268,11 +268,6 @@ fn write_impl(

268 let jobs = &mut cx.jobs;

269 let doc = doc_mut!(cx.editor);

270

271− if let Some(ref path) = path {

272− doc.set_path(Some(path.as_ref().as_ref()))

273− .context("invalid filepath")?;

274− }

275−

276 if doc.path().is_none() {

271 if doc.path().is_none() {

277 bail!("cannot write a buffer without a filename");

272 bail!("cannot write a buffer without a filename");

278 }

273 }

@@ -292,7 +287,7 @@ fn write_impl(

292 None

287 None

293 };

288 };

294

289

295− doc.format_and_save(fmt, force)?;

290+ doc.format_and_save(fmt, path.map(AsRef::as_ref), force)?;

296

291

297 if path.is_some() {

292 if path.is_some() {

298 let id = doc.id();

293 let id = doc.id();

@@ -607,7 +602,7 @@ fn write_all_impl(

607 None

602 None

608 };

603 };

609

604

610− doc.format_and_save(fmt, force)?;

605+ doc.format_and_save::<_, PathBuf>(fmt, None, force)?;

611 }

606 }

612

607

613 if quit {

608 if quit {

helix-term/tests/test/write.rs+59−2

@@ -35,7 +35,7 @@ async fn test_write() -> anyhow::Result<()> {

35 Ok(())

36}

38−#[tokio::test]

38+#[tokio::test(flavor = "multi_thread")]

39async fn test_write_quit() -> anyhow::Result<()> {

40 let mut file = tempfile::NamedTempFile::new()?;

@@ -129,7 +129,64 @@ async fn test_write_fail_mod_flag() -> anyhow::Result<()> {

129}

130

131#[tokio::test]

132−#[ignore]

132+async fn test_write_new_path() -> anyhow::Result<()> {

133+ let mut file1 = tempfile::NamedTempFile::new().unwrap();

134+ let mut file2 = tempfile::NamedTempFile::new().unwrap();

135+

136+ test_key_sequences(

137+ &mut Application::new(

138+ Args {

139+ files: vec![(file1.path().to_path_buf(), Position::default())],

140+ ..Default::default()

141+ },

142+ Config::default(),

143+ )?,

144+ vec![

145+ (

146+ Some("ii can eat glass, it will not hurt me<ret><esc>:w<ret>"),

147+ Some(&|app| {

148+ let doc = doc!(app.editor);

149+ assert!(!app.editor.is_err());

150+ assert_eq!(file1.path(), doc.path().unwrap());

151+ }),

152+ ),

153+ (

154+ Some(&format!(":w {}<ret>", file2.path().to_string_lossy())),

155+ Some(&|app| {

156+ let doc = doc!(app.editor);

157+ assert!(!app.editor.is_err());

158+ assert_eq!(file2.path(), doc.path().unwrap());

159+ assert!(app.editor.document_by_path(file1.path()).is_none());

160+ }),

161+ ),

162+ ],

163+ false,

164+ )

165+ .await?;

166+

167+ file1.as_file_mut().flush()?;

168+ file1.as_file_mut().sync_all()?;

169+ file2.as_file_mut().flush()?;

170+ file2.as_file_mut().sync_all()?;

171+

172+ let mut file1_content = String::new();

173+ file1.as_file_mut().read_to_string(&mut file1_content)?;

174+ assert_eq!(

175+ helpers::platform_line("i can eat glass, it will not hurt me\n"),

176+ file1_content

177+ );

178+

179+ let mut file2_content = String::new();

180+ file2.as_file_mut().read_to_string(&mut file2_content)?;

181+ assert_eq!(

182+ helpers::platform_line("i can eat glass, it will not hurt me\n"),

183+ file2_content

184+ );

185+

186+ Ok(())

187+}

188+

189+#[tokio::test]

133async fn test_write_fail_new_path() -> anyhow::Result<()> {

190async fn test_write_fail_new_path() -> anyhow::Result<()> {

134 let file = helpers::new_readonly_tempfile()?;

191 let file = helpers::new_readonly_tempfile()?;

135

192

helix-view/src/document.rs+37−15

@@ -91,6 +91,7 @@ impl Serialize for Mode {

91pub struct DocumentSaveEvent {

92 pub revision: usize,

93 pub doc_id: DocumentId,

94+ pub path: PathBuf,

94}

95}

96pub type DocumentSaveEventResult = Result<DocumentSaveEvent, anyhow::Error>;

97pub type DocumentSaveEventResult = Result<DocumentSaveEvent, anyhow::Error>;

@@ -512,41 +513,61 @@ impl Document {

512 Some(fut.boxed())

513 Some(fut.boxed())

513 }

514 }

514

515

516+ pub fn save<P: Into<PathBuf>>(

517+ &mut self,

518+ path: Option<P>,

519+ force: bool,

515− pub fn save(&mut self, force: bool) -> Result<(), anyhow::Error> {

520+ ) -> Result<(), anyhow::Error> {

516− self.save_impl::<futures_util::future::Ready<_>>(None, force)

521+ self.save_impl::<futures_util::future::Ready<_>, _>(None, path, force)

517 }

522 }

518

523

519− pub fn format_and_save(

524+ pub fn format_and_save<F, P>(

520 &mut self,

525 &mut self,

521− formatting: Option<

526+ formatting: Option<F>,

522− impl Future<Output = Result<Transaction, FormatterError>> + 'static + Send,

527+ path: Option<P>,

523− >,

524 force: bool,

528 force: bool,

525− ) -> anyhow::Result<()> {

529+ ) -> anyhow::Result<()>

530+ where

531+ F: Future<Output = Result<Transaction, FormatterError>> + 'static + Send,

532+ P: Into<PathBuf>,

533+ {

526− self.save_impl(formatting, force)

534+ self.save_impl(formatting, path, force)

527 }

535 }

528

536

529− // TODO: impl Drop to handle ensuring writes when closed

530 /// The `Document`'s text is encoded according to its encoding and written to the file located

537 /// The `Document`'s text is encoded according to its encoding and written to the file located

531 /// at its `path()`.

538 /// at its `path()`.

532 ///

539 ///

533 /// If `formatting` is present, it supplies some changes that we apply to the text before saving.

540 /// If `formatting` is present, it supplies some changes that we apply to the text before saving.

534− fn save_impl<F: Future<Output = Result<Transaction, FormatterError>> + 'static + Send>(

541+ fn save_impl<F, P>(

535 &mut self,

542 &mut self,

536 formatting: Option<F>,

543 formatting: Option<F>,

544+ path: Option<P>,

537 force: bool,

545 force: bool,

538− ) -> Result<(), anyhow::Error> {

546+ ) -> Result<(), anyhow::Error>

547+ where

548+ F: Future<Output = Result<Transaction, FormatterError>> + 'static + Send,

549+ P: Into<PathBuf>,

550+ {

539 if self.save_sender.is_none() {

551 if self.save_sender.is_none() {

540 bail!("saves are closed for this document!");

552 bail!("saves are closed for this document!");

541 }

553 }

542

554

543 // we clone and move text + path into the future so that we asynchronously save the current

555 // we clone and move text + path into the future so that we asynchronously save the current

544 // state without blocking any further edits.

556 // state without blocking any further edits.

545−

546 let mut text = self.text().clone();

557 let mut text = self.text().clone();

547− let path = self.path.clone().expect("Can't save with no path set!");

548− let identifier = self.identifier();

549

558

559+ let path = match path {

560+ Some(path) => helix_core::path::get_canonicalized_path(&path.into())?,

561+ None => {

562+ if self.path.is_none() {

563+ bail!("Can't save with no path set!");

564+ }

565+

566+ self.path.as_ref().unwrap().clone()

567+ }

568+ };

569+

570+ let identifier = self.identifier();

550 let language_server = self.language_server.clone();

571 let language_server = self.language_server.clone();

551

572

552 // mark changes up to now as saved

573 // mark changes up to now as saved

@@ -586,12 +607,13 @@ impl Document {

586 }

607 }

587 }

608 }

588

609

589− let mut file = File::create(path).await?;

610+ let mut file = File::create(&path).await?;

590 to_writer(&mut file, encoding, &text).await?;

611 to_writer(&mut file, encoding, &text).await?;

591

612

592 let event = DocumentSaveEvent {

613 let event = DocumentSaveEvent {

593 revision: current_rev,

614 revision: current_rev,

594 doc_id,

615 doc_id,

616+ path,

595 };

617 };

596

618

597 if let Some(language_server) = language_server {

619 if let Some(language_server) = language_server {