History

fix(gix-config): don't indent a value that starts on a continuation line

`git` discards whitespace for as long as the value it has accumulated is still empty, so the indentation of the line a value *starts* on is insignificant — whether that is the line with the `=` on it, or a later line reached through a `\` continuation. `parse::from_bytes::value()` only applied this to the first line: after a continuation it set `value_start` to the first byte of the next line, so the indentation became part of the value. Given this `.git/config`: ```ini [alias] lg = \ log --oneline --graph ``` `git config --get alias.lg` reports `log --oneline --graph`, while `File::raw_value("alias.lg")` reported `\t\tlog --oneline --graph`. The leading tabs are silently carried into whatever runs the alias, and the same happens to any continued value written in this shape, such as a `core.pager` set over two lines. `value()` now emits that indentation as `Event::Whitespace` for as long as every chunk seen so far has been empty — the very sequence that `Event::ValueNotDone` already documents ("A `Newline` event usually follows, followed by either `ValueDone`, `Whitespace`, or another `ValueNotDone`"). Because the events keep the bytes, `to_bstring()` still round-trips the file unchanged, and every consumer already ignores `Whitespace` when concatenating value chunks. Indentation that follows actual content is untouched, matching `git`: `k = abc\` + `\n\tdef` stays `abc\tdef`, and multi-line aliases that begin on the `=` line keep their interior whitespace.

jaideeppyne committed 2 months ago

Commit a441bb21 parent e3a6fa13 files · +104 −0Browse files

Unified · Split

3 files
  1. gix-config/src/parse/from_bytes/mod.rs +19 −0
  2. gix-config/src/parse/from_bytes/tests.rs +58 −0
  3. gix-config/tests/config/file/access/read_only.rs +27 −0
gix-config/src/parse/from_bytes/mod.rs+19−0
@@ -309,6 +309,8 @@ fn config_value(backing: &[u8], i: &mut &[u8], dispatch: &mut dyn FnMut(Event)) · 308 unmodified lines
309/// Double quotes toggle quoted mode for comment handling. Supported escapes are309/// Double quotes toggle quoted mode for comment handling. Supported escapes are
310/// backslash followed by `n`, `t`, `\`, `b`, `"`, LF, or CRLF. Line continuations310/// backslash followed by `n`, `t`, `\`, `b`, `"`, LF, or CRLF. Line continuations
311/// emit [`Event::ValueNotDone`], the continuation newline, and finally [`Event::ValueDone`].311/// emit [`Event::ValueNotDone`], the continuation newline, and finally [`Event::ValueDone`].
312/// As long as nothing was accumulated yet, the indentation of a continuation line is insignificant
313/// and emitted as [`Event::Whitespace`], just like the whitespace right after the `=`.
312/// If the value ends with a trailing backslash at EOF, it is emitted as314/// If the value ends with a trailing backslash at EOF, it is emitted as
313/// [`Event::ValueNotDone`] followed directly by an empty [`Event::ValueDone`].315/// [`Event::ValueNotDone`] followed directly by an empty [`Event::ValueDone`].
314/// Otherwise a single [`Event::Value`] is emitted with trailing ASCII whitespace316/// Otherwise a single [`Event::Value`] is emitted with trailing ASCII whitespace
@@ -323,6 +325,8 @@ fn value(backing: &[u8], i: &mut &[u8], dispatch: &mut dyn FnMut(Event)) -> Pars · 8 unmodified lines
323 let mut is_in_quotes = false;325 let mut is_in_quotes = false;
324 // Set after a line continuation so the final chunk is emitted as `ValueDone`.326 // Set after a line continuation so the final chunk is emitted as `ValueDone`.
325 let mut partial_value_found = false;327 let mut partial_value_found = false;
328 // Cleared once a continuation chunk contributed bytes, to know if leading whitespace is still insignificant.
329 let mut value_is_empty_so_far = true;
326 330
327 while cursor < input.len() {331 while cursor < input.len() {
328 match input[cursor] {332 match input[cursor] {
@@ -357,11 +361,26 @@ fn value(backing: &[u8], i: &mut &[u8], dispatch: &mut dyn FnMut(Event)) -> Pars · 28 unmodified lines
357 b'\n' => {361 b'\n' => {
358 partial_value_found = true;362 partial_value_found = true;
359 let value = input[value_start..escape_index].as_bstr();363 let value = input[value_start..escape_index].as_bstr();
364 value_is_empty_so_far &= value.is_empty();
360 dispatch(Event::ValueNotDone(Span::new(backing, value)));365 dispatch(Event::ValueNotDone(Span::new(backing, value)));
361 let nl_start = escape_index + 1;366 let nl_start = escape_index + 1;
362 let nl = input[nl_start..nl_start + consumed].as_bstr();367 let nl = input[nl_start..nl_start + consumed].as_bstr();
363 dispatch(Event::Newline(Span::new(backing, nl)));368 dispatch(Event::Newline(Span::new(backing, nl)));
364 cursor += 1;369 cursor += 1;
370 if value_is_empty_so_far {
371 // Git keeps discarding whitespace for as long as the value it accumulated is empty,
372 // so a value that only starts on a continuation line must not pick up that line's
373 // indentation, just like it doesn't pick up the whitespace right after the `=`.
374 let indent_len = input[cursor..]
375 .iter()
376 .take_while(|b| **b == b' ' || **b == b'\t')
377 .count();
378 if indent_len != 0 {
379 let indent = input[cursor..cursor + indent_len].as_bstr();
380 dispatch(Event::Whitespace(Span::new(backing, indent)));
381 cursor += indent_len;
382 }
383 }
365 value_start = cursor;384 value_start = cursor;
366 value_end = None;385 value_end = None;
367 }386 }
gix-config/src/parse/from_bytes/tests.rs+58−0
@@ -726,6 +726,7 @@ mod value_continuation { · 725 unmodified lines
726 726
727 use crate::parse::tests::util::{727 use crate::parse::tests::util::{
728 OwnedEvent, newline_custom_event, newline_event, own_event, value_done_event, value_not_done_event,728 OwnedEvent, newline_custom_event, newline_event, own_event, value_done_event, value_not_done_event,
729 whitespace_event,
729 };730 };
730 731
731 pub fn value<'a>(input: &'a [u8], events: &mut Vec<OwnedEvent>) -> Result<(&'a [u8], ()), ()> {732 pub fn value<'a>(input: &'a [u8], events: &mut Vec<OwnedEvent>) -> Result<(&'a [u8], ()), ()> {
@@ -784,6 +785,63 @@ mod value_continuation { · 52 unmodified lines
784 );785 );
785 }786 }
786 787
788 #[test]
789 fn indentation_of_the_line_a_value_starts_on_is_insignificant() {
790 for (input, newline) in [
791 (&b"\\\n world"[..], newline_event()),
792 (&b"\\\r\n world"[..], newline_custom_event("\r\n")),
793 ] {
794 let mut events = Vec::new();
795 assert_eq!(value(input, &mut events).unwrap().0, b"");
796 assert_eq!(
797 events,
798 vec![
799 value_not_done_event(""),
800 newline,
801 whitespace_event(" "),
802 value_done_event("world")
803 ],
804 "Git discards whitespace for as long as the value it accumulated is empty, \
805 so a value that only starts after a continuation begins at `world`"
806 );
807 }
808 }
809
810 #[test]
811 fn indentation_is_insignificant_until_the_value_has_content() {
812 let mut events = Vec::new();
813 assert_eq!(value(b"\\\n \\\n world", &mut events).unwrap().0, b"");
814 assert_eq!(
815 events,
816 vec![
817 value_not_done_event(""),
818 newline_event(),
819 whitespace_event(" "),
820 value_not_done_event(""),
821 newline_event(),
822 whitespace_event(" "),
823 value_done_event("world")
824 ],
825 "a continuation line that is entirely whitespace keeps the value empty, \
826 so the next line's indentation is insignificant as well"
827 );
828
829 let mut events = Vec::new();
830 assert_eq!(value(b"\\\n hello\\\n world", &mut events).unwrap().0, b"");
831 assert_eq!(
832 events,
833 vec![
834 value_not_done_event(""),
835 newline_event(),
836 whitespace_event(" "),
837 value_not_done_event("hello"),
838 newline_event(),
839 value_done_event(" world")
840 ],
841 "once the value has content, indentation is part of it, just like interior whitespace"
842 );
843 }
844
787 #[test]845 #[test]
788 fn complex_continuation_with_leftover_comment() {846 fn complex_continuation_with_leftover_comment() {
789 let mut events = Vec::new();847 let mut events = Vec::new();
gix-config/tests/config/file/access/read_only.rs+27−0
@@ -484,6 +484,33 @@ fn multi_line_value_with_empty_continuation_line() { · 483 unmodified lines
484 );484 );
485}485}
486 486
487#[test]
488fn multi_line_value_starting_on_a_continuation_line_is_not_indented() {
489 for config in [
490 "[core]\n\tk = \\\n\tabc\n",
491 "[core]\n\tk = \\\n abc\n",
492 "[core]\n\tk =\\\n\t\tabc\n",
493 "[core]\n\tk = \\\n\t\\\n\tabc\n",
494 "[core]\r\n\tk = \\\r\n\tabc\r\n",
495 ] {
496 let file = File::try_from(config).unwrap();
497 assert_eq!(
498 file.raw_value("core.k").unwrap(),
499 bstring("abc"),
500 "Git reports `abc` for {config:?}: it discards whitespace for as long as the value it \
501 accumulated is empty, so the indentation of the line the value starts on is insignificant"
502 );
503 }
504
505 let config = "[core]\n\tk = abc\\\n\tdef\n";
506 let file = File::try_from(config).unwrap();
507 assert_eq!(
508 file.raw_value("core.k").unwrap(),
509 bstring("abc\tdef"),
510 "…whereas indentation that follows actual content is part of the value, like any interior whitespace"
511 );
512}
513
487#[test]514#[test]
488fn overrides_with_implicit_booleans_work_in_single_section() {515fn overrides_with_implicit_booleans_work_in_single_section() {
489 let config = r#"516 let config = r#"

Keyboard shortcuts

General

Show keyboard shortcuts
?
Focus the current search
/

Issues and pull requests

Open filters
f

Issues, pull requests and Inbox

Select next item
j↓
Select previous item
k↑
Open selected item
Enter
Dismiss filters or clear selection
Esc

Repository code

Go to file
t