Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add insert-final-newline config option #8157

Merged
merged 6 commits into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions book/src/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ Its settings will be merged with the configuration directory `config.toml` and t
| `text-width` | Maximum line length. Used for the `:reflow` command and soft-wrapping if `soft-wrap.wrap-at-text-width` is set | `80` |
| `workspace-lsp-roots` | Directories relative to the workspace root that are treated as LSP roots. Should only be set in `.helix/config.toml` | `[]` |
| `default-line-ending` | The line ending to use for new documents. Can be `native`, `lf`, `crlf`, `ff`, `cr` or `nel`. `native` uses the platform's native line ending (`crlf` on Windows, otherwise `lf`). | `native` |
| `insert-final-newline` | Whether to automatically insert a trailing line-ending on write if missing | `true` |

### `[editor.statusline]` Section

Expand Down
70 changes: 43 additions & 27 deletions helix-term/src/commands/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::job::Job;
use super::*;

use helix_core::fuzzy::fuzzy_match;
use helix_core::{encoding, shellwords::Shellwords};
use helix_core::{encoding, line_ending, shellwords::Shellwords};
use helix_view::document::DEFAULT_LANGUAGE_NAME;
use helix_view::editor::{Action, CloseError, ConfigEvent};
use serde_json::Value;
Expand Down Expand Up @@ -330,12 +330,16 @@ fn write_impl(
path: Option<&Cow<str>>,
force: bool,
) -> anyhow::Result<()> {
let editor_auto_fmt = cx.editor.config().auto_format;
let config = cx.editor.config();
let jobs = &mut cx.jobs;
let (view, doc) = current!(cx.editor);
let path = path.map(AsRef::as_ref);

let fmt = if editor_auto_fmt {
if config.insert_final_newline {
insert_final_newline(doc, view);
}

let fmt = if config.auto_format {
doc.auto_format().map(|fmt| {
let callback = make_format_callback(
doc.id(),
Expand All @@ -359,6 +363,16 @@ fn write_impl(
Ok(())
}

fn insert_final_newline(doc: &mut Document, view: &mut View) {
let text = doc.text();
if line_ending::get_line_ending(&text.slice(..)).is_none() {
let eof = Selection::point(text.len_chars());
let insert = Transaction::insert(text, &eof, doc.line_ending.as_str().into());
doc.apply(&insert, view.id);
doc.append_changes_to_history(view);
}
}

fn write(
cx: &mut compositor::Context,
args: &[Cow<str>],
Expand Down Expand Up @@ -658,11 +672,10 @@ pub fn write_all_impl(
write_scratch: bool,
) -> anyhow::Result<()> {
let mut errors: Vec<&'static str> = Vec::new();
let auto_format = cx.editor.config().auto_format;
let config = cx.editor.config();
let jobs = &mut cx.jobs;
let current_view = view!(cx.editor);

// save all documents
let saves: Vec<_> = cx
.editor
.documents
Expand Down Expand Up @@ -693,32 +706,35 @@ pub fn write_all_impl(
current_view.id
};

let fmt = if auto_format {
doc.auto_format().map(|fmt| {
let callback = make_format_callback(
doc.id(),
doc.version(),
target_view,
fmt,
Some((None, force)),
);
jobs.add(Job::with_callback(callback).wait_before_exiting());
})
} else {
None
};
Some((doc.id(), target_view))
})
.collect();

if fmt.is_none() {
return Some(doc.id());
}
for (doc_id, target_view) in saves {
let doc = doc_mut!(cx.editor, &doc_id);

if config.insert_final_newline {
insert_final_newline(doc, view_mut!(cx.editor, target_view));
}

let fmt = if config.auto_format {
doc.auto_format().map(|fmt| {
let callback = make_format_callback(
doc_id,
doc.version(),
target_view,
fmt,
Some((None, force)),
);
jobs.add(Job::with_callback(callback).wait_before_exiting());
})
} else {
None
})
.collect();
};

// manually call save for the rest of docs that don't have a formatter
for id in saves {
cx.editor.save::<PathBuf>(id, None, force)?;
if fmt.is_none() {
cx.editor.save::<PathBuf>(doc_id, None, force)?;
}
}

if !errors.is_empty() && !force {
Expand Down
125 changes: 122 additions & 3 deletions helix-term/tests/test/commands/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ async fn test_buffer_close_concurrent() -> anyhow::Result<()> {
)
.await?;

helpers::assert_file_has_content(file.as_file_mut(), &RANGE.end().to_string())?;
helpers::assert_file_has_content(file.as_file_mut(), &platform_line(&RANGE.end().to_string()))?;

Ok(())
}
Expand Down Expand Up @@ -209,7 +209,7 @@ async fn test_write_concurrent() -> anyhow::Result<()> {

let mut file_content = String::new();
file.as_file_mut().read_to_string(&mut file_content)?;
assert_eq!(RANGE.end().to_string(), file_content);
assert_eq!(platform_line(&RANGE.end().to_string()), file_content);

Ok(())
}
Expand Down Expand Up @@ -424,13 +424,132 @@ async fn test_write_utf_bom_file() -> anyhow::Result<()> {
Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn test_write_insert_final_newline_added_if_missing() -> anyhow::Result<()> {
let mut file = tempfile::NamedTempFile::new()?;
let mut app = helpers::AppBuilder::new()
.with_file(file.path(), None)
.with_input_text("#[h|]#ave you tried chamomile tea?")
.build()?;

test_key_sequence(&mut app, Some(":w<ret>"), None, false).await?;

helpers::assert_file_has_content(
file.as_file_mut(),
&helpers::platform_line("have you tried chamomile tea?\n"),
)?;

Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn test_write_insert_final_newline_unchanged_if_not_missing() -> anyhow::Result<()> {
let mut file = tempfile::NamedTempFile::new()?;
let mut app = helpers::AppBuilder::new()
.with_file(file.path(), None)
.with_input_text(&helpers::platform_line("#[t|]#en minutes, please\n"))
.build()?;

test_key_sequence(&mut app, Some(":w<ret>"), None, false).await?;

helpers::assert_file_has_content(
file.as_file_mut(),
&helpers::platform_line("ten minutes, please\n"),
)?;

Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn test_write_insert_final_newline_unchanged_if_missing_and_false() -> anyhow::Result<()> {
let mut file = tempfile::NamedTempFile::new()?;
let mut app = helpers::AppBuilder::new()
.with_config(Config {
editor: helix_view::editor::Config {
insert_final_newline: false,
..Default::default()
},
..Default::default()
})
.with_file(file.path(), None)
.with_input_text("#[t|]#he quiet rain continued through the night")
.build()?;

test_key_sequence(&mut app, Some(":w<ret>"), None, false).await?;

helpers::assert_file_has_content(
file.as_file_mut(),
"the quiet rain continued through the night",
)?;

Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn test_write_all_insert_final_newline_add_if_missing_and_modified() -> anyhow::Result<()> {
let mut file1 = tempfile::NamedTempFile::new()?;
let mut file2 = tempfile::NamedTempFile::new()?;
let mut app = helpers::AppBuilder::new()
.with_file(file1.path(), None)
.with_input_text("#[w|]#e don't serve time travelers here")
.build()?;

test_key_sequence(
&mut app,
Some(&format!(
":o {}<ret>ia time traveler walks into a bar<esc>:wa<ret>",
file2.path().to_string_lossy()
)),
None,
false,
)
.await?;

helpers::assert_file_has_content(
file1.as_file_mut(),
&helpers::platform_line("we don't serve time travelers here\n"),
)?;

helpers::assert_file_has_content(
file2.as_file_mut(),
&helpers::platform_line("a time traveler walks into a bar\n"),
)?;

Ok(())
}

#[tokio::test(flavor = "multi_thread")]
async fn test_write_all_insert_final_newline_do_not_add_if_unmodified() -> anyhow::Result<()> {
let mut file = tempfile::NamedTempFile::new()?;
let mut app = helpers::AppBuilder::new()
.with_file(file.path(), None)
.build()?;

file.write_all(b"i lost on Jeopardy!")?;
file.rewind()?;

test_key_sequence(&mut app, Some(":wa<ret>"), None, false).await?;

helpers::assert_file_has_content(file.as_file_mut(), "i lost on Jeopardy!")?;

Ok(())
}

async fn edit_file_with_content(file_content: &[u8]) -> anyhow::Result<()> {
let mut file = tempfile::NamedTempFile::new()?;

file.as_file_mut().write_all(&file_content)?;

helpers::test_key_sequence(
&mut helpers::AppBuilder::new().build()?,
&mut helpers::AppBuilder::new()
.with_config(Config {
editor: helix_view::editor::Config {
insert_final_newline: false,
..Default::default()
},
..Default::default()
})
.build()?,
Some(&format!(":o {}<ret>:x<ret>", file.path().to_string_lossy())),
None,
true,
Expand Down
2 changes: 1 addition & 1 deletion helix-term/tests/test/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ pub fn assert_file_has_content(file: &mut File, content: &str) -> anyhow::Result

let mut file_content = String::new();
file.read_to_string(&mut file_content)?;
assert_eq!(content, file_content);
assert_eq!(file_content, content);

Ok(())
}
Expand Down
6 changes: 3 additions & 3 deletions helix-term/tests/test/splits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ async fn test_split_write_quit_all() -> anyhow::Result<()> {
)
.await?;

helpers::assert_file_has_content(file1.as_file_mut(), "hello1")?;
helpers::assert_file_has_content(file2.as_file_mut(), "hello2")?;
helpers::assert_file_has_content(file3.as_file_mut(), "hello3")?;
helpers::assert_file_has_content(file1.as_file_mut(), &platform_line("hello1"))?;
helpers::assert_file_has_content(file2.as_file_mut(), &platform_line("hello2"))?;
helpers::assert_file_has_content(file3.as_file_mut(), &platform_line("hello3"))?;

Ok(())
}
Expand Down
3 changes: 3 additions & 0 deletions helix-view/src/editor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,8 @@ pub struct Config {
pub workspace_lsp_roots: Vec<PathBuf>,
/// Which line ending to choose for new documents. Defaults to `native`. i.e. `crlf` on Windows, otherwise `lf`.
pub default_line_ending: LineEndingConfig,
/// Whether to automatically insert a trailing line-ending on write if missing. Defaults to `true`.
pub insert_final_newline: bool,
/// Enables smart tab
pub smart_tab: Option<SmartTabConfig>,
}
Expand Down Expand Up @@ -842,6 +844,7 @@ impl Default for Config {
completion_replace: false,
workspace_lsp_roots: Vec::new(),
default_line_ending: LineEndingConfig::default(),
insert_final_newline: true,
smart_tab: Some(SmartTabConfig::default()),
}
}
Expand Down