edit plugin: make temporary file reflect --set CLI arguments

[niacdoial]

Description

when using the edit plugin during import, make the user aware that any --set fields
will be overwritten, and pre-fill said fields.

(by --set fields, I mean for instance album in beet imp "--set=album=Greatester Hits" ./tracks-for-which-the-titles-will-be-detected-wrong/)

To Do

• Documentation.
• Changelog. (awaiting review)
• Tests. Not sure if it is possible to test UI-only changes

[sampsyo]

Thanks for the interesting approach! It looks like the idea here is to insert to a block comment above the main YAML doc with some reminders to the user. Maybe we should think a little bit about the UI implications. For example:

• Will people get tired of seeing this after the first time? Especially if you edit many albums, the set of fields will always be the same for the same import session.
• Printing this before the YAML document means that people have to move the cursor downward to start editing. Maybe having stuff like this below, like a git commit message, would be more usable?
• Perhaps it would be helpful to separate the two categories of read-only fields so users know why they're appearing?

[sampsyo]

Is there a specific reason that you used a separate event handler here? It also seems like we could fetch the list of fields directly in the edit_object method.

[niacdoial]

no specific reason, just bad planning (I initially thought that the set fields were only accessible in the session object, which is out of scope in the edit_object method, and then didn't simplify the code when I found out it wasn't the case)

[niacdoial]

In terms of UI, I made changes that look at the first two points you pointed out.

• One thing for the second point is: if there are a lot of tracks in a single file, we risk having the message be way offscreen.
  Maybe the message should be on top when the file is 'long enough' for us to assume people would either (1) just use the edit command to look into what properties are currently set, and in that case do not need to move the cursor around too much, or (2), look at all tracks and maybe edit them, which means they would have scrolled down anyway.

• for the third point, I believe the two categories you are talking about are (1) the 'set fields' and (2) id and path, which need to be determined later in the importing process.
  In that case, I see why you would warn about editing path. For id, however, it's obvious (to me) that editing it is pointless, because the negative value makes it clear that it's a placeholder.

Actually, it might be less friction to not show that warning message in the file itself, but detect if a read-only/placeholder field has been edited, and warn about that in the terminal? ...It might cause the user to edit a field for 20+ tracks, only to be told that work was useless.

I'll need to think more about which solution introduces the least amount of friction.

[niacdoial]

for now, I've settled upon:

• have the --set fields be commented out (with an extra end-of-line comment saying they are read-only)
• have the placeholder field removed from the editable text
• have a warning message if changes are detected anyway (with proper explanations as why the user's changes need to be discarded)

I just realised that there is no way to proper way to unset a field now, just set it to be blank. Let me fix that.

[niacdoial]

actually, don't merge just yet, there might be a really bad breaking bug that escaped the existing tests, and discards user edits in... some situations?
I'll have to dig.
(also, editing the imported track to a known album/albumartist combo, then choosing "merge albums" then "use as-is" makes the --set flags overwrite the other tracks of the merged albums? I'll work on a MWE and proper bug report soon-ish)