Adjust get_selected_contents behavior

[30350n]

Draft PR to enable the fix for jj #3702 and jj #3846 in jj #4078.

The logic in get_selected_contents generally seems really confusing to me and these changes probably don't help mitigating that. From what I can tell, it doesn't break existing tests though, neither in this repo, nor in jj (in combination with jj #4078).

The main problems seem to be:

1. The general structure of get_selected_contents Going through the sections one-by-one makes it hard to differentiate between Absent and Unchanged SelectedContents.
2. The way scm-record handles (doesn't handle) file mode changes I feel like the client shouldn't be responsible for manually adding FileMode sections at all. Ideally this part wouldn't be exposed at all, but just be handled by scm-record internally.

[arxanas]

Going through the sections one-by-one makes it hard to differentiate

I agree that the file mode is treated weirdly and interacts poorly with the rest of the context/data model But rather than extend the logic, as done in this PR, I think we should remove it and simply report a file mode transition (selected Section::FileMode) to the caller, who can then decide how to handle it:

• We should delete File::file_mode.
• We should just return the file mode transition (if any) directly as a part of SelectedContents, and remove any logic that sets SelectedContents::Absent/SelectedContents::Present based on the file mode.
  • The caller can decide how to handle the file mode transition, if any.
  • There shouldn't be any special handling for FileMode::absent() when we're done.

I feel like the client shouldn't be responsible for manually adding FileMode sections at all. Ideally this part wouldn't be exposed at all, but just be handled by scm-record internally.

There are already some competing designs (such as how to handle partially-committed changes in conjunction with file mode changes), which indicates that scm-record might not be able to establish an authoritative design that works for all callers. Therefore, I think that scm-record should focus on rendering the UI and leave the semantic interpretation to the caller. (In accordance with that, we should also move get_selected_contents to the helpers module, as it imposes a semantic interpretation on the UI contents.)

[arxanas]

We should just return the file mode transition (if any) directly as a part of SelectedContents

Or we can keep SelectedContents as it is, and just update get_file_mode to not check the File::file_mode, and have the caller continue to handle that separately.