Documentation update to map.rs as part of tracking issue #107540#154513
Documentation update to map.rs as part of tracking issue #107540#154513KaiPetzke wants to merge 1 commit intorust-lang:mainfrom
Conversation
As Part of tracking issue rust-lang#107540, I submit a comment fix (the comments describing the method CursorMut::insert_after_unchecked() should say "before" instead of "after". This can be seen for example from the comments for CursorMutKey::insert_after_unchecked(), which are correct. I further submit a suggestion to make the comments before all Cursor insert methods slightly more verbose to explain the before/after in the names of those methods: insert_after() inserts the new item effectively after the cursor, thus positioning the cursor itself before the new element. I hope the extra verbosity will help to clarify the actual behavior of these methods. No actual code is updated.
rustbot
added
S-waiting-on-review
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
| /// After the insertion the cursor will be pointing at the gap before the | ||
| /// newly inserted element. | ||
| /// newly inserted element, effectively inserting the new element after the | ||
| /// current cursor position. |
There was a problem hiding this comment.
Hm, I'm not sure this isn't ambiguous too. I think maybe we can rephrase more strongly? Something like:
The cursor remains pointed in the gap after the same element as before the insertion. The next element after the cursor is the newly inserted element.
There was a problem hiding this comment.
Yes, an even stronger rephrasing is fine. I just think, we shouldn't use the wording "before the insertion" though, and use the words "before" and "after" exclusively for spatial context (relative ordering in the map) not for timely context (before/after calling a function) to avoid misreading.
A very elaborate version would be:
The new element is inserted into the gap between the two
elements before and after the cursor. When the insertion
completes, the cursor is positioned to point at the gap between
the element before the cursor and the newly inserted element,
so that the newly inserted element will be located after the cursor.
Or another suggestion:
The new element is inserted after the cursor into the gap,
that the cursor is pointing at. When the insertion completes,
the cursor will be pointing at the gap between the item, that
has originally been before the cursor, and the newly inserted
element after it.
But I am also fine with your suggestion.
There was a problem hiding this comment.
I think both of those phrasings leave potentially ambiguous the actual behavior, in particular around the cursor not moving. I think the "remains pointed" phrasing from mine helps make it clear relatively succinctly that the cursor has not moved.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
3 participants
As Part of tracking issue #107540, I submit a document fix (the comments describing the method
CursorMut::insert_after_unchecked()should say "before" instead of "after". This can be seen for example from the comments forCursorMutKey::insert_after_unchecked(), which are correct.I further submit a suggestion to make the comments before all Cursor insert methods slightly more verbose to explain the before/after in the names of those methods: insert_after() inserts the new item effectively after the cursor, thus positioning the cursor itself before the new element. I hope the extra verbosity will help to clarify the actual behavior of these methods.
No actual code is updated.