[polucifier]

Adds the Delete shortcut for removing items in the Data Catalog, and fixes the
Layers pane one, which has been in the code for the entire history of the
project but never worked.

This covers the delete shortcuts from #5425 that were suggested in #7423. The
Data Catalog and the Layers pane are in the same PR because both were asked for
and the changes are small and independent. They are kept in separate commits so
they are trivial to split if you prefer.

Data Catalog

keys	selection	action
Delete	map (raster, vector, 3D raster)	delete the map, after confirmation
Delete	mapset, or several mapsets	delete, after one confirmation
Delete	project, or several projects	delete, after confirmation
Delete	GRASS database node	remove it from the catalog only, the data stays on disk
Ctrl+Shift+Delete	GRASS database node	delete the database from disk

The conditions mirror the context menu items, including the ones that disable
them, so the shortcuts never remove more than the menus offer:

• the database currently in use cannot be removed from the catalog
• a map inside a space-time dataset is not touched, the menu only offers to
  unregister it
• a mixed selection, or several database nodes, does nothing
• a node being renamed is not deleted, the keys go to the label editor
• nothing happens when no item is selected

Layers pane

LayerTree.OnKeyUp binds EVT_KEY_UP and dispatches WXK_DELETE to the layer
manager, and both are already present in the parent of
c730ae7e6,
the 2011-11-24 reorganisation that moved layertree.py to its current path,
gui/wxpython/lmgr/layertree.py. So nothing was ever missing from the handler,
it simply never ran.

Each layer item carries an options button, which is a child of the tree, and the
Delete key event is delivered to that button rather than to the tree, so a
binding on the tree never sees it. This does not depend on clicking the button:
selecting a layer row with the mouse and pressing Delete behaves exactly the
same, and in both cases the key never reaches LayerTree.OnKeyUp, so the
shortcut has never been usable from the Layers pane.

The shortcut is now bound on the button as well, through a
_createLayerButton() helper shared by AddLayer and RecreateItem so the two
call sites cannot drift apart. Removing a layer destroys the button which is
dispatching the event, so the removal is deferred with CallAfter, the same way
the button click handler already avoids destroying itself on some platforms.
event.Skip() is called on every path, so that the button keeps handling space
and enter itself.

Testing

Both panes were tested manually in the wxGUI, going through every selection
type listed above, including the cases that must be refused, several items
selected at once, a node being renamed, and Delete with nothing selected.

Not included

Space-time datasets are left out on purpose. The context menu offers both
delete dataset only and delete dataset and maps as separate items, and a
single key cannot choose between them. It would be good to get an opinion on how
this should behave before implementing it.

Related findings, deliberately left out of this PR

I didn't find an existing issue for any of these; they all came out of testing this change.

• Arrow key navigation does not work in the Layers pane.
  CustomTreeCtrl.__init__ binds EVT_KEY_DOWN to self.OnKeyDown, which
  GRASS's LayerTree overrides with a bare event.Skip(), so the widget's own
  key handling never runs.
• After a layer is removed, the options buttons of the remaining rows move next
  to the layer text instead of staying at the right edge of the row. This
  reproduces on the current release with two or more layers, both through the
  button's context menu and through the Delete shortcut.
• _isCurrent() returns (True, True, True) unconditionally unless
  _restricted is set, and SetRestriction is never called, so
  item.Enable(currentMapset) is currently a no-op in the popup builders. The
  shortcut reproduces the same condition so that it matches the menu, but the
  underlying issue looks worth its own report.

Addresses the delete shortcuts proposed in #5425 for the Data Catalog and the
Layers pane.

[petrasovaa]

@saket0187 would you be able to review this?

[saket0187]

It looks good in general. Here are a few suggestions:

• Delete on a database node removes it from the catalog tree without any confirmation; if possible, please add one as for maps and mapsets.
• Not for this PR: The Yes/No buttons are in different positions in the delete map and in other dialogs, and I deleted a mapset by mistake because of it :( , this is worth a follow-up PR.
• About space-time datasets: maybe Delete = "Delete dataset only" (maps are kept, the safer choice) and Ctrl+Shift+Delete = "Delete dataset and maps", like the database case? Both already ask for confirmation.

[saket0187]

Optional: since this only needs "exactly one item, and it's a database", it could check that directly.

Suggested change

	Nodes below the database in the tree populate the selected database as
	well, so the node type has to be checked explicitly. Only a database
	node itself can be removed.
	"""
	if (
	self.mixed
	or self.selected_layer[0]
	or self.selected_stds[0]
	or self.selected_mapset[0]
	or self.selected_location[0]
	or len(self.selected_grassdb) != 1
	):
	return None
	return self.selected_grassdb[0]
	"""
	selected = self.GetSelected()
	if len(selected) == 1 and selected[0].data["type"] == "grassdb":
	return selected[0]
	return None

[saket0187]

Not related to this PR, but this check has no effect: SetRestriction() is never called, so _restricted is always False and _isCurrent() always returns True. @petrasovaa should the restriction code be removed in a follow-up, or is it meant to come back?

[petrasovaa]

Remove in follow up, and it doesn't need to be added here.

[saket0187]

Suggested change

	if event.ControlDown() and event.ShiftDown():
	# Deleting a database from disk cannot be undone by adding it
	# back to the catalog, so it needs an extra modifier
	if self._selectedGrassDb() and not self._restricted:
	self.OnDeleteGrassDb(None)
	return
	else:
	handler = self._getRemoveHandler()
	if handler:
	handler(None)
	return
	modifiers = event.GetModifiers()
	if modifiers == wx.MOD_CONTROL | wx.MOD_SHIFT:
	# Deleting a database from disk cannot be undone by adding it
	# back to the catalog, so it needs an extra modifier
	if self._selectedGrassDb() and not self._restricted:
	self.OnDeleteGrassDb(None)
	return
	elif modifiers == wx.MOD_NONE:
	handler = self._getRemoveHandler()
	if handler:
	handler(None)
	return

Any Delete with other modifiers works (e.g. Shift+Delete) here. Maybe check the modifiers exactly with event.GetModifiers()?

[saket0187]

Suggested change

	Delete selected layer
	Remove selected map layer(s)

[petrasovaa]

I would suggest removing this, it's rather rare and not expected.

[petrasovaa]

• About space-time datasets: maybe Delete = "Delete dataset only" (maps are kept, the safer choice) and Ctrl+Shift+Delete = "Delete dataset and maps", like the database case? Both already ask for confirmation.

I am not sure whether Ctrl+Shift+Delete is all that useful, user would have to know about it, while simple delete is obvious. It seems the action would be rare enough to just select it from context menu. But feel free to disagree, I know many people use shortcuts more than I.

But I agree with the Delete = "Delete dataset only" (maps are kept, the safer choice)

[saket0187]

Suggested change

	if event.GetKeyCode() != wx.WXK_DELETE:
	if event.GetKeyCode() not in {wx.WXK_DELETE, wx.WXK_NUMPAD_DELETE}:

Consider handling wx.WXK_NUMPAD_DELETE as well, so the Delete key on a numeric keypad is supported.