Add new action for bulk-modifying all tasks currently in view (addresses #240)

[kevinstadler]

Here we go, a new user-facing action {ACTION_TASK_MODIFY_ALL} which bulk-modifies all tasks currently visible in vit by means of a new 'modify_multiple' operation. This new internal operation is not limited to this one use case, but can also be used to bulk-modify sets of tasks that are selected by other means (e.g. manually by the user, as was discussed in #240).

Note that I have gone for the option of adding a new operation 'modify_multiple' just because I didn't want to touch the original 'modify' operation -- except for some extra safety checks whether the number of affected tasks is still the same and a different status bar message at the end, these two operations execute the same command. So instead of having two operations with different names it would also be possible to stick to just one 'modify' operation and distinguish the use cases internally based on their arguments (in particular whether a uuid is passed or a pair of target/ntasks).

I realised some of the filter-concatenation functionality of my earlier PR #289 was already implemented in task.py, so I've made use of the existing methods instead of adding my own.

[thehunmonkgroup]

Looking fairly good on first inspection, just a few minor requests.

[kevinstadler]

Looking good, should I add something to the documentation as well? (Where?)

[thehunmonkgroup]

I say we make it a top-ticket item at https://github.com/vit-project/vit#features

Bulk edit all tasks in current view ?

There's no great place yet to add a more detailed description. The two best candidates seem to be:

• https://github.com/vit-project/vit/blob/2.x/FAQ.md
• https://github.com/vit-project/vit/blob/2.x/vit/config/config.sample.ini

Probably the sample config is the best place, since you can offer an example of the token being used.

Thoughts?

[kevinstadler]

I'm not sure it's that crucial of a feature to warrant explicit mention on the front page, maybe mentioning it in CHANGES.md is enough. Any system in place for accumulating a changelog prior to a new release?

Adding it to the sample config sounds good, how would you feel about replacing

vit/vit/config/config.sample.ini

Lines 194 to 196 in 1d59db8

	# For capital letter keybindings, use the letter directly:
	# D = {ACTION_TASK_DONE}

with

# For capital letter keybindings, use the letter directly:
#   m = {ACTION_TASK_MODIFY}
#   M = {ACTION_TASK_MODIFY_ALL}

?

[thehunmonkgroup]

I'm not sure it's that crucial of a feature to warrant explicit mention on the front page, maybe mentioning it in CHANGES.md is enough. Any system in place for accumulating a changelog prior to a new release?

Yes, see scripts/generate-changelog-entries.sh -- but we won't do that until we're ready for a release.

If this feature was 'bulk edit', then it would definitely warrant mention in the features. The currrent implementation isn't quite there yet. It's probably best to wait.

Adding it to the sample config sounds good, how would you feel about replacing

Just add it, we can have multiple examples there.

Also, because it's a registered action, it appears in the output of vit --list-actions, which is the user's source of truth for available actions.

I think that's enough for now.

[kevinstadler]

If this feature was 'bulk edit', then it would definitely warrant mention in the features. The currrent implementation isn't quite there yet. It's probably best to wait.

To be consistent with taskwarrior terminology, bulk modification (not editing, which is completely interactive) is simply modification with a filter expression that matches more than one task. Subject to the rc.bulk setting this might force interactive confirmation, but other than that is completely automatic. If we go by that terminology, then the current PR already implements bulk modification.

#240 implies a second feature, which I would call manually selecting tasks (for the purpose of calling bulk modification on them), but that is IMHO functionality that combines two features that should be treated separately (because each have their own uses): the internal bulk modification operation on one hand, and a new UI feature like #291 on the other.

[kevinstadler]

Just added a binding (including a comment regarding the rc.bulk setting) to config.sample.ini.

I also just realised from writing the above that my choice of 'modify_multiple' for the internal operation name was maybe not ideal/fully consistent with taskwarrior. Should I rename it to 'modify_bulk' instead?

[thehunmonkgroup]

I also just realised from writing the above that my choice of 'modify_multiple' for the internal operation name was maybe not ideal/fully consistent with taskwarrior. Should I rename it to 'modify_bulk' instead?

Yep, sounds good.

Thanks for for refresher on bulk modifications, and it does convince me more that 'bulk modifications' should be in the main feature list -- toss it in there! We can adjust/enhance it if/when the other multi-task modification feature lands.

[thehunmonkgroup]

We're very, very close now on the code review!

[thehunmonkgroup]

OK, ran some quick tests, we could be there. I have two questions:

1. Do we want to provide a default keybinding for ACTION_TASK_MODIFY_ALL ?
   M is already taken by ACTION_LIST_SCREEN_MIDDLE, and I don't really want to break backward compat there.
   Is there another key or key combo that would work?
2. Are we sure we want to go with ACTION_TASK_MODIFY_ALL for the action token? I worry that 'all' could be misleading. ACTION_TASK_MODIFY_BULK or ACTION_TASK_MODIFY_CURRENT_REPORT perhaps? I'm slowing down here because I'd prefer to get it right now and not risk changing it later, which would break some config, or leave cruft in the code to support a legacy token.

[kevinstadler]

1. no strong thoughts, it's a powerful/potentially destructive feature so maybe bind to some more out-of-the-way keybinding? Actually come to think of it the command is in a way similar to vi's global regex substitute, i.e. could model the binding after :s/......./g?
2. so I think 'bulk modification' is what is happening underneath (hence the name of the internal operation), but any user-facing action actually does two things, namely (1) select a set of tasks and then (2) bulk-modify them. I therefore think the user-facing action name should contain explicit information about which tasks are selected for modification, and not just an ambiguous _BULK. ACTION_TASK_MODIFY_CURRENT_REPORT sounds to me like modifying the report rather than the tasks inside it. The problem is that within task/vit-lingo there is no name for "the currently visible list of tasks", which is the (very intuitive but nameless) joint product of context filters + report filters + extra filters. So maybe it is best to steer clear of using any names that already have a meaning inside task/vit and decide on a new term for that specific 'list' that we actually look at. Some candidates I can think of:

• ACTION_TASK_MODIFY_VISIBLE
• ACTION_TASK_MODIFY_LISTED
• ACTION_TASK_MODIFY_VIEW
  Thoughts?

[thehunmonkgroup]

1. I'd like to make some kind of decision on this before we merge, mostly because if there is a default keybinding, then having one in the sample config doesn't make any sense. I'm in favor of a default keybinding, but one that would be impossible to fat finger. Maybe Ctrl+Shift+m or Ctrl+Alt+m or Shift+Alt+m? I'd say of those three I favor the first, but could be persuaded otherwise.
2. This one is tough, for sure. Following your same logic, ACTION_TASK_MODIFY_VISIBLE could be misleading, if the report has more tasks than can fit on screen. ACTION_TASK_MODIFY_VIEW suffers from the same potential confusion that ACTION_TASK_MODIFY_CURRENT_REPORT does. ACTION_TASK_MODIFY_LISTED has potential. What is actually happening is that we're bulk modifying all tasks that pass through all the currently applied filters. ACTION_TASK_MODIFY_CURRENTLY_FILTERED_TASKS? That's more descriptive, but beginning to get a little unwieldy...