fix(gui): edit or delete part of a tag without losing the rest - #129
Open
dustenhubbard wants to merge 1 commit into
Open
fix(gui): edit or delete part of a tag without losing the rest#129dustenhubbard wants to merge 1 commit into
dustenhubbard wants to merge 1 commit into
Conversation
Correcting a tag meant deleting it and typing it again. Reproducing that
turned up a separate reason on each surface that edits tags:
- `MultiInput.remove` popped the last row wherever the caret was, so
fixing the first of several tags deleted every row below it. It now
removes the row being edited, and keeps one row alive by clearing it
rather than leaving a field with nothing to type into.
- The tag rows and the palette's Tags cell were built straight from a
set, so a tag moved between openings and a part-finished edit resumed
on the wrong row. Both are sorted now.
- The palette's Tags cell was parsed with `text.split(", ")`, which
turns the "axon, " left behind by deleting "spine" into two tags, the
second empty, and turns an untagged trace's empty cell into a single
empty tag. `parseTags` splits on the comma, strips, and drops empties.
- `Series.editObjectAttributes` passed `add_tags=True` to
`Section.editTraceAttributes`, which iterates the incoming set and
adds each element, so an edited set could add a tag but never drop
one. It takes `add_tags` now, defaulting to True so no other caller
changes, and the object list passes False when the dialog was
actually showing that object's tags.
Adds a tests/ directory, which the repo did not have.
Closes #119
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Makes an existing tag editable in place, so correcting one no longer means deleting it and retyping the whole thing. Reproducing the report turned up a separate cause on each surface that edits tags.
Object list.
editObjectAttributescallededitTraceAttributes(add_tags=True)unconditionally, and that branch only ever adds, so deleting a row and confirming did nothing. It now takesadd_tags, defaulting toTrueso no caller changes meaning, and the object list sets it from whether the dialog was actually showing the selection's tags. Replacing on a multi-object selection would discard tags the user was never shown.Trace attributes dialog.
MultiInput's "-" ranself.inputs.pop(), removing the last row wherever the caret was, so fixing the first of several tags meant retyping the rest. It now removes the row being edited, which is why both buttons areQt.NoFocus. Rows came from a set, so they moved between openings; they are sorted now.Trace palette. The Tags cell is parsed with
text.split(", "), so removingspinefromaxon, spineleaves an empty second tag, and"".split(", ")is[""]-- confirmingEdit all palettes...untouched gave every untagged trace an empty tag. Splitting on the comma and dropping empties fixes both.Includes 32 tests. The repo has no
tests/directory, so this adds one.Closes #119