manipulator: Remove a saved profession.

[kindaro]

I was annoyed when I accidentally saved a generic Fish Cleaner profession and could not find an in-game option to remove it.

My suggestion is that we add such an option to manipulator's menu.

I drafted the necessary changes here. It seems to work, so I will be living with it from now on. If there is approval, I can make a pull request, but I will need some mentorship on code quality and cross platform support, since I have very little C++ skill.

[lethosor]

Isn't there already a screen with a list of professions (to load them)? It'd probably be easier to add a delete option to that screen than making an entirely new screen. I'm not sure adding a whole new line to the manipulator options is worth it either (although it'll probably be necessary at some point...). I can leave comments on that commit if you like - there are probably a couple DFHack-specific things that could be improved.

[kindaro]

@lethosor It is most desirable that you do comment.

I did not add a delete option to the load screen for two reasons:

• I like to think of it as a "standard dialogue" that could be factored out and shared between many plugins to provide consistent behaviour.

• If a user wants to remove a saved profession, why would they use "load" option? It will have to be relabeled to "load or delete" or even "load/manage" then.

• • *

I noticed, a while after creating the present issue, that stockpiles may be saved and loaded too — would be nice if the interface was consistent. They actually suffer from the absence of removal option, as well.

I wonder if we can think of some generic solution that can be factored out and used both in manipulator and in the stockpile save/load feature. It would offer to:

• Save the current configuration.
• Observe the list of existing configurations and choose which one to load.
• The same, but to delete.

It will also need some framework for manipulating the persistent storage. Currently, the plugin author is choosing what files to create, which is unsafe. _(What if we accidentally write over an existing file? What if the name to be saved, supplied by the user, contains ../?) If we start adding the removal feature, it is yet another opportunity to slip. So, some thought should be given to standardizing the actions a plugin may do on a file system.

One possibility is to give each plugin a directory it may create and remove any files inside at will, but in such a fashion that paths with .. in them are not accepted.

It is a bit of a long shot, I admit.

[lethosor]

The stockpiles plugin uses a Lua file for its save/load UI - there's also some sanitization to deal with the safety issues you mentioned. It could be modularized/reused in manipulator, but manipulator doesn't use Lua at all currently, so that would take some additional effort.

My reasoning for suggesting the delete option in the "load" screen was:

• Deleting isn't a very common operation (this is the first suggestion I've seen for it - maybe some people delete files by hand when they want to?). It would take up more UI space, which I'm not sure is worth it - I don't have an issue with relabeling the option.
• Usually, people would want to delete files because they're taking up space in the "load" screen. Making them go through a separate screen seems tedious to me - it could turn a procedure like "load -> delete A -> confirm -> select B" into "load -> back -> delete -> select A -> confirm -> back -> load -> select B".

I do agree that standardizing things is good. I'm not sure it's necessary for stockpiles and manipulator at this moment, but if they both support things like searching and deletion, it would be a good idea. Consistent rejection of dangerous filenames is also good.

I'll comment on the commit here.