Add config for default line ending

[CptPotato]

I've finally gotten around to giving this a shot.

This adds a key to the editor config that allows specifying the default line ending for new documents. I added variants for native, lf, crlf, ff, cr or nel. native uses the platform's native line ending (crlf on Windows, otherwise lf). The default is native, so the out-of-the-box behavior remains unchanged.

One thing I noticed is that the enum naming is a little odd - Crlf vs. LF. But I kept the existing variant names.

closes #4378

[CptPotato]

I'm not quite sure what I should do with this test:

helix/helix-view/src/document.rs

Lines 1392 to 1398 in 68fc109

	#[test]
	fn test_line_ending() {
	assert_eq!(
	Document::default().text().to_string(),
	DEFAULT_LINE_ENDING.as_str()
	);
	}

There's no way to create a document without specifying the desired line ending now. Adding one would make this test kinda useless.

[pascalkuthe]

I'm not quite sure what I should do with this test:

helix/helix-view/src/document.rs

Lines 1392 to 1398 in 68fc109

	#[test]
	fn test_line_ending() {
	assert_eq!(
	Document::default().text().to_string(),
	DEFAULT_LINE_ENDING.as_str()
	);
	}

There's no way to create a document without specifying the desired line ending now. Adding one would make this test kinda useless.

I think test mostly ensures that new documents just contain a newline ending and nothing else so I would just use LineEnding::LF and assert that the document text is equal to \n.

This test wasn't testing the correct detection if the default line ending anyway (as it was just comparing to the same constant used in the implememtation) so no need to start now

[CptPotato]

I think test mostly ensures that new documents just contain a newline ending and nothing else so I would just use LineEnding::LF and assert that the document text is equal to \n.

Alright, the test is fixed now. I added Crlf to the test as well, to have a test case with two characters.

Edit: actually, since tests are run on all platforms I'm now using LineEnding::native() in all tests. I also missed some tests at first since I wasn't running cargo with the --workspace flag, though it should be good to go now. I hope all the force pushes don't get confusing 👀

[g-re-g]

I'm probably missing something here, but if the return of this function is going to be both constant and baked in at compile time, is there a reason it can't be a regular const like before? Perhaps with a different name:

#[cfg(target_os = "windows")]
pub const NATIVE_LINE_ENDING: LineEnding = LineEnding::Crlf;
#[cfg(not(target_os = "windows"))]
pub const NATIVE_LINE_ENDING: LineEnding = LineEnding::LF;

[CptPotato]

Yup, you're right, it technically does the same thing. I initially removed DEFAULT_LINE_ENDING because because it made refactoring much easier - i.e. everything that depends on that constant needs to be changed, since it's not constant anymore.

Apart from that, having it tied to LineEnding made more sense to me so I added it there. Though, I don't have a strong preference for this, so if there's a consensus I can change it back to a constant.

[g-re-g]

Oh word I gotcha. If you'd like it to still be a constant and also tied to the LineEnding enum you can actually put constants in an enum's impl:

impl LineEnding {
    #[cfg(target_os = "windows")]
    pub const NATIVE_LINE_ENDING: LineEnding = LineEnding::Crlf;
    #[cfg(not(target_os = "windows"))]
    pub const NATIVE_LINE_ENDING: LineEnding = LineEnding::LF;


    ...

And then refer to it like a constant qualified by the enum:

let doc = Document::from(rope, None, LineEnding::NATIVE_LINE_ENDING);

[pascalkuthe]

If it's not absolutely necessary I would prefer if we could keep the constant same as it is currently in master. I think it's readable and used elsewhere and changes like these introduce loads of merge conflicts in other PRs. If there is no functional reason that this needs to be refactored (which I don't think there is) then I would prefer to just keep it as in master an only change the type of the constant.

[CptPotato]

If it's not absolutely necessary I would prefer if we could keep the constant same as it is currently in master. I think it's readable and used elsewhere and changes like these introduce loads of merge conflicts in other PRs. If there is no functional reason that this needs to be refactored (which I don't think there is) then I would prefer to just keep it as in master an only change the type of the constant.

I'll do that then, thanks for the feedback. Though it's worth noting that this will still cause merge conflicts due to the rename. Keeping the same constant name DEFAULT_LINE_ENDING is out of question in my opinion because it is not the default line ending (anymore), but the platform's native one.

[CptPotato]

Just changed it back to a const.

[CptPotato]

Just merged current master due to conflicts.

Since there's a config parameter for document creation I'm now using that to get the default line ending config. Though I'm not sure if I'm accessing it correctly using config.load().default_line_ending.into()

[pascalkuthe]

Just merged current master due to conflicts.

Since there's a config parameter for document creation I'm now using that to get the default line ending config. Though I'm not sure if I'm accessing it correctly using config.load().default_line_ending.into()

That usage is correct 👍 The config field of Document acts always the same as the editor config field so use it the same as you would use that. I added that field in #5420 as otherwise we would need to add a config paramter to a ton of functions which would have been very nosy and accessing config trough the document is actually something that often comes in handy

[CptPotato]

Squashed and rebased onto latest master