0f3c10a · helix · git.iko.soy

git.iko.soy/helix

Fix initial selection of Document in new view

When a new View of a Document is created, a default cursor of 0, 0 is created, and it does not get normalized to a single width cursor until at least one movement of the cursor happens. This appears to have no practical negative effect that I could find, but it makes tests difficult to work with, since the initial selection is not what you expect it to be. This changes the initial selection of a new View to be the width of the first grapheme in the text.

commit0f3c10a021bbe79e20bde1f55b87465edeec476d authorSkyler Hawthorne <skyler@dead10ck.com> dateWed 16 Mar 2022, 23:34 (5 years ago) parent502d329

10 files changed+173−65

compare with master

unifiedsplit

.gitignore−1 helix-core/src/auto_pairs.rs+2−12 helix-core/src/selection.rs+9−1 helix-loader/Cargo.toml−1 helix-loader/src/lib.rs+3−1 helix-term/src/application.rs+21 helix-term/src/commands.rs+12−5 helix-term/tests/integration.rs+86−26 helix-view/src/document.rs+33−1 helix-view/src/editor.rs+7−17

.gitignore−1

@@ -1,6 +1,5 @@

11target

22.direnv

33helix-term/rustfmt.toml

4−helix-syntax/languages/

54result

65runtime/grammars

helix-core/src/auto_pairs.rs+2−12

@@ -1,9 +1,7 @@

11//! When typing the opening character of one of the possible pairs defined below,

22//! this module provides the functionality to insert the paired closing character.

4−use crate::{

5− graphemes, movement::Direction, Range, Rope, RopeGraphemes, Selection, Tendril, Transaction,

6−};

4+use crate::{graphemes, movement::Direction, Range, Rope, Selection, Tendril, Transaction};

75use std::collections::HashMap;

97use log::debug;

@@ -149,14 +147,6 @@ fn prev_char(doc: &Rope, pos: usize) -> Option<char> {

149147 doc.get_char(pos - 1)

150148}

151149

152−fn is_single_grapheme(doc: &Rope, range: &Range) -> bool {

153− let mut graphemes = RopeGraphemes::new(doc.slice(range.from()..range.to()));

154− let first = graphemes.next();

155− let second = graphemes.next();

156− debug!("first: {:#?}, second: {:#?}", first, second);

157− first.is_some() && second.is_none()

158−}

159−

160150/// calculate what the resulting range should be for an auto pair insertion

161151fn get_next_range(

162152 doc: &Rope,

@@ -189,8 +179,8 @@ fn get_next_range(

189179 );

190180 }

191181

192− let single_grapheme = is_single_grapheme(doc, start_range);

193182 let doc_slice = doc.slice(..);

183+ let single_grapheme = start_range.is_single_grapheme(doc_slice);

194184

195185 // just skip over graphemes

196186 if len_inserted == 0 {

helix-core/src/selection.rs+9−1

@@ -8,7 +8,7 @@ use crate::{

88 prev_grapheme_boundary,

99 },

1010 movement::Direction,

11− Assoc, ChangeSet, RopeSlice,

11+ Assoc, ChangeSet, RopeGraphemes, RopeSlice,

1212};

1313use smallvec::{smallvec, SmallVec};

1414use std::borrow::Cow;

@@ -339,6 +339,14 @@ impl Range {

339339 pub fn cursor_line(&self, text: RopeSlice) -> usize {

340340 text.char_to_line(self.cursor(text))

341341 }

342+

343+ /// Returns true if this Range covers a single grapheme in the given text

344+ pub fn is_single_grapheme(&self, doc: RopeSlice) -> bool {

345+ let mut graphemes = RopeGraphemes::new(doc.slice(self.from()..self.to()));

346+ let first = graphemes.next();

347+ let second = graphemes.next();

348+ first.is_some() && second.is_none()

349+ }

342350}

343351

344352impl From<(usize, usize)> for Range {

helix-loader/Cargo.toml−1

@@ -20,7 +20,6 @@ toml = "0.5"

2020etcetera = "0.4"

2121tree-sitter = "0.20"

2222once_cell = "1.12"

23−

2423log = "0.4"

2524

2625# TODO: these two should be on !wasm32 only

helix-loader/src/lib.rs+3−1

@@ -13,7 +13,9 @@ pub fn runtime_dir() -> std::path::PathBuf {

1313

1414 if let Ok(dir) = std::env::var("CARGO_MANIFEST_DIR") {

1515 // this is the directory of the crate being run by cargo, we need the workspace path so we take the parent

16− return std::path::PathBuf::from(dir).parent().unwrap().join(RT_DIR);

16+ let path = std::path::PathBuf::from(dir).parent().unwrap().join(RT_DIR);

17+ log::debug!("runtime dir: {}", path.to_string_lossy());

18+ return path;

1719 }

1820

1921 const RT_DIR: &str = "runtime";

helix-term/src/application.rs+21

@@ -55,8 +55,29 @@ pub struct Application {

5555 lsp_progress: LspProgressMap,

5656}

5757

58+#[cfg(feature = "integration")]

59+fn setup_integration_logging() {

60+ // Separate file config so we can include year, month and day in file logs

61+ let _ = fern::Dispatch::new()

62+ .format(|out, message, record| {

63+ out.finish(format_args!(

64+ "{} {} [{}] {}",

65+ chrono::Local::now().format("%Y-%m-%dT%H:%M:%S%.3f"),

66+ record.target(),

67+ record.level(),

68+ message

69+ ))

70+ })

71+ .level(log::LevelFilter::Debug)

72+ .chain(std::io::stdout())

73+ .apply();

74+}

75+

5876impl Application {

5977 pub fn new(args: Args, config: Config) -> Result<Self, Error> {

78+ #[cfg(feature = "integration")]

79+ setup_integration_logging();

80+

6081 use helix_view::editor::Action;

6182

6283 let config_dir = helix_loader::config_dir();

helix-term/src/commands.rs+12−5

@@ -2094,10 +2094,17 @@ fn insert_mode(cx: &mut Context) {

20942094 let (view, doc) = current!(cx.editor);

20952095 enter_insert_mode(doc);

20962096

2097− let selection = doc

2098− .selection(view.id)

2099− .clone()

2100− .transform(|range| Range::new(range.to(), range.from()));

2097+ log::trace!(

2098+ "entering insert mode with sel: {:?}, text: {:?}",

2099+ doc.selection(view.id),

2100+ doc.text().to_string()

2101+ );

2102+

2103+ let selection = doc.selection(view.id).clone().transform(|range| {

2104+ let new_range = Range::new(range.to(), range.from());

2105+ new_range

2106+ });

2107+

21012108 doc.set_selection(view.id, selection);

21022109}

21032110

@@ -2444,8 +2451,8 @@ fn normal_mode(cx: &mut Context) {

24442451 graphemes::prev_grapheme_boundary(text, range.to()),

24452452 )

24462453 });

2447− doc.set_selection(view.id, selection);

24482454

2455+ doc.set_selection(view.id, selection);

24492456 doc.restore_cursor = false;

24502457 }

24512458}

helix-term/tests/integration.rs+86−26

@@ -2,9 +2,9 @@

22mod integration {

33 use std::path::PathBuf;

5− use helix_core::{syntax::AutoPairConfig, Position, Selection, Tendril, Transaction};

5+ use helix_core::{syntax::AutoPairConfig, Position, Selection, Transaction};

66 use helix_term::{application::Application, args::Args, config::Config};

7− use helix_view::{current, doc, input::parse_macro};

7+ use helix_view::{doc, input::parse_macro};

99 use crossterm::event::{Event, KeyEvent};

1010 use indoc::indoc;

@@ -25,14 +25,14 @@ mod integration {

2525 let mut app =

2626 app.unwrap_or_else(|| Application::new(Args::default(), Config::default()).unwrap());

2727

28− let (view, doc) = current!(app.editor);

28+ let (view, doc) = helix_view::current!(app.editor);

29+ let sel = doc.selection(view.id).clone();

2930

31+ // replace the initial text with the input text

3032 doc.apply(

31− &Transaction::insert(

32− doc.text(),

33− &Selection::single(1, 0),

34− Tendril::from(&test_case.in_text),

35− )

33+ &Transaction::change_by_selection(&doc.text(), &sel, |_| {

34+ (0, doc.text().len_chars(), Some((&test_case.in_text).into()))

35+ })

3636 .with_selection(test_case.in_selection.clone()),

3737 view.id,

3838 );

@@ -80,12 +80,12 @@ mod integration {

8080 Args::default(),

8181 Config::default(),

8282 TestCase {

83− in_text: String::new(),

83+ in_text: "\n".into(),

8484 in_selection: Selection::single(0, 1),

8585 // TODO: fix incorrect selection on new doc

86− in_keys: String::from("ihello world<esc>hl"),

87− out_text: String::from("hello world\n"),

88− out_selection: Selection::single(11, 12),

86+ in_keys: "ihello world<esc>".into(),

87+ out_text: "hello world\n".into(),

88+ out_selection: Selection::single(12, 11),

8989 },

9090 )?;

9191

@@ -93,16 +93,74 @@ mod integration {

9393 }

9494

9595 #[tokio::test]

96− async fn auto_pairs_basic() -> anyhow::Result<()> {

96+ async fn insert_mode_cursor_position() -> anyhow::Result<()> {

9797 test_key_sequence_text_result(

9898 Args::default(),

9999 Config::default(),

100100 TestCase {

101101 in_text: String::new(),

102+ in_selection: Selection::single(0, 0),

103+ in_keys: "i".into(),

104+ out_text: String::new(),

105+ out_selection: Selection::single(0, 0),

106+ },

107+ )?;

108+

109+ test_key_sequence_text_result(

110+ Args::default(),

111+ Config::default(),

112+ TestCase {

113+ in_text: "\n".into(),

114+ in_selection: Selection::single(0, 1),

115+ in_keys: "i".into(),

116+ out_text: "\n".into(),

117+ out_selection: Selection::single(1, 0),

118+ },

119+ )?;

120+

121+ test_key_sequence_text_result(

122+ Args::default(),

123+ Config::default(),

124+ TestCase {

125+ in_text: "\n".into(),

102126 in_selection: Selection::single(0, 1),

103− in_keys: String::from("i(<esc>hl"),

104− out_text: String::from("()\n"),

105− out_selection: Selection::single(1, 2),

127+ in_keys: "i<esc>i".into(),

128+ out_text: "\n".into(),

129+ out_selection: Selection::single(1, 0),

130+ },

131+ )?;

132+

133+ Ok(())

134+ }

135+

136+ #[tokio::test]

137+ async fn insert_to_normal_mode_cursor_position() -> anyhow::Result<()> {

138+ test_key_sequence_text_result(

139+ Args::default(),

140+ Config::default(),

141+ TestCase {

142+ in_text: "\n".into(),

143+ in_selection: Selection::single(0, 1),

144+ in_keys: "i".into(),

145+ out_text: "\n".into(),

146+ out_selection: Selection::single(1, 0),

147+ },

148+ )?;

149+

150+ Ok(())

151+ }

152+

153+ #[tokio::test]

154+ async fn auto_pairs_basic() -> anyhow::Result<()> {

155+ test_key_sequence_text_result(

156+ Args::default(),

157+ Config::default(),

158+ TestCase {

159+ in_text: "\n".into(),

160+ in_selection: Selection::single(0, 1),

161+ in_keys: "i(<esc>".into(),

162+ out_text: "()\n".into(),

163+ out_selection: Selection::single(2, 1),

106164 },

107165 )?;

108166

@@ -116,11 +174,11 @@ mod integration {

116174 ..Default::default()

117175 },

118176 TestCase {

119− in_text: String::new(),

177+ in_text: "\n".into(),

120178 in_selection: Selection::single(0, 1),

121− in_keys: String::from("i(<esc>hl"),

122− out_text: String::from("(\n"),

123− out_selection: Selection::single(1, 2),

179+ in_keys: "i(<esc>".into(),

180+ out_text: "(\n".into(),

181+ out_selection: Selection::single(2, 1),

124182 },

125183 )?;

126184

@@ -136,15 +194,17 @@ mod integration {

136194 },

137195 Config::default(),

138196 TestCase {

139− in_text: String::from("void foo() {}"),

140− in_selection: Selection::single(12, 13),

141− in_keys: String::from("i<ret><esc>"),

142− out_text: String::from(indoc! {r#"

197+ in_text: "void foo() {}\n".into(),

198+ in_selection: Selection::single(13, 12),

199+ in_keys: "i<ret><esc>".into(),

200+ out_text: indoc! {r#"

143201 void foo() {

144202

145203 }

146− "#}),

147− out_selection: Selection::single(15, 16),

204+ "#}

205+ .trim_start()

206+ .into(),

207+ out_selection: Selection::single(16, 15),

148208 },

149209 )?;

150210

helix-view/src/document.rs+33−1

@@ -1,5 +1,6 @@

11use anyhow::{anyhow, bail, Context, Error};

22use helix_core::auto_pairs::AutoPairs;

3+use helix_core::Range;

34use serde::de::{self, Deserialize, Deserializer};

45use serde::Serialize;

56use std::cell::Cell;

@@ -83,7 +84,7 @@ impl Serialize for Mode {

8384pub struct Document {

8485 pub(crate) id: DocumentId,

8586 text: Rope,

86− pub(crate) selections: HashMap<ViewId, Selection>,

87+ selections: HashMap<ViewId, Selection>,

8788

8889 path: Option<PathBuf>,

8990 encoding: &'static encoding::Encoding,

@@ -637,6 +638,37 @@ impl Document {

637638 .insert(view_id, selection.ensure_invariants(self.text().slice(..)));

638639 }

639640

641+ /// Find the origin selection of the text in a document, i.e. where

642+ /// a single cursor would go if it were on the first grapheme. If

643+ /// the text is empty, returns (0, 0).

644+ pub fn origin(&self) -> Range {

645+ if self.text().len_chars() == 0 {

646+ return Range::new(0, 0);

647+ }

648+

649+ Range::new(0, 1).grapheme_aligned(self.text().slice(..))

650+ }

651+

652+ /// Reset the view's selection on this document to the

653+ /// [origin](Document::origin) cursor.

654+ pub fn reset_selection(&mut self, view_id: ViewId) {

655+ let origin = self.origin();

656+ self.set_selection(view_id, Selection::single(origin.anchor, origin.head));

657+ }

658+

659+ /// Initializes a new selection for the given view if it does not

660+ /// already have one.

661+ pub fn ensure_view_init(&mut self, view_id: ViewId) {

662+ if self.selections.get(&view_id).is_none() {

663+ self.reset_selection(view_id);

664+ }

665+ }

666+

667+ /// Remove a view's selection from this document.

668+ pub fn remove_view(&mut self, view_id: ViewId) {

669+ self.selections.remove(&view_id);

670+ }

671+

640672 /// Apply a [`Transaction`] to the [`Document`] to change its text.

641673 fn apply_impl(&mut self, transaction: &Transaction, view_id: ViewId) -> bool {

642674 let old_doc = self.text().clone();

helix-view/src/editor.rs+7−17

@@ -32,12 +32,12 @@ use anyhow::{bail, Error};

3232

3333pub use helix_core::diagnostic::Severity;

3434pub use helix_core::register::Registers;

35+use helix_core::Position;

3536use helix_core::{

3637 auto_pairs::AutoPairs,

3738 syntax::{self, AutoPairConfig},

3839 Change,

3940};

40−use helix_core::{Position, Selection};

4141use helix_dap as dap;

4242

4343use serde::{ser::SerializeMap, Deserialize, Deserializer, Serialize, Serializer};

@@ -645,11 +645,8 @@ impl Editor {

645645 view.offset = Position::default();

646646

647647 let doc = self.documents.get_mut(&doc_id).unwrap();

648+ doc.ensure_view_init(view.id);

648649

649− // initialize selection for view

650− doc.selections

651− .entry(view.id)

652− .or_insert_with(|| Selection::point(0));

653650 // TODO: reuse align_view

654651 let pos = doc

655652 .selection(view.id)

@@ -719,9 +716,7 @@ impl Editor {

719716 Action::Load => {

720717 let view_id = view!(self).id;

721718 let doc = self.documents.get_mut(&id).unwrap();

722− if doc.selections().is_empty() {

723− doc.set_selection(view_id, Selection::point(0));

724− }

719+ doc.ensure_view_init(view_id);

725720 return;

726721 }

727722 Action::HorizontalSplit | Action::VerticalSplit => {

@@ -736,7 +731,7 @@ impl Editor {

736731 );

737732 // initialize selection for view

738733 let doc = self.documents.get_mut(&id).unwrap();

739− doc.set_selection(view_id, Selection::point(0));

734+ doc.ensure_view_init(view_id);

740735 }

741736 }

742737

@@ -769,7 +764,7 @@ impl Editor {

769764 Ok(self.new_file_from_document(action, Document::from(rope, Some(encoding))))

770765 }

771766

772− // ???

767+ // ??? possible use for integration tests

773768 pub fn open(&mut self, path: PathBuf, action: Action) -> Result<DocumentId, Error> {

774769 let path = helix_core::path::get_canonicalized_path(&path)?;

775770 let id = self.document_by_path(&path).map(|doc| doc.id);

@@ -791,12 +786,7 @@ impl Editor {

791786 pub fn close(&mut self, id: ViewId) {

792787 let view = self.tree.get(self.tree.focus);

793788 // remove selection

794− self.documents

795− .get_mut(&view.doc)

796− .unwrap()

797− .selections

798− .remove(&id);

799−

789+ self.documents.get_mut(&view.doc).unwrap().remove_view(id);

800790 self.tree.remove(id);

801791 self._refresh();

802792 }

@@ -871,7 +861,7 @@ impl Editor {

871861 let view = View::new(doc_id, self.config().gutters.clone());

872862 let view_id = self.tree.insert(view);

873863 let doc = self.documents.get_mut(&doc_id).unwrap();

874− doc.set_selection(view_id, Selection::point(0));

864+ doc.ensure_view_init(view_id);

875865 }

876866

877867 self._refresh();