FEATURE: 197009
FIXED-IN: 19.08.0
Details
Details
- Reviewers
ngraham elvisangelaccio - Group Reviewers
Dolphin
Right click - rename tab, enter some name to rename tab
Right click - rename tab, clear lineedit in dialog, ok to reset name to default
Diff Detail
Diff Detail
- Repository
- R318 Dolphin
- Branch
- arcpatch-D20052
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 10175 Build 10193: arc lint + arc unit
Comment Actions
Thanks, this generally works very nicely. I found a big bug though: when you rename a tab and then move it to another position on the tab bar, the tab in its old position no longer updates its name when the user navigates around in that tab, and the renamed tab inappropriately does update its name as the user navigates around.
| src/dolphintabbar.cpp | ||
|---|---|---|
| 164 | Spaces around the equals sign | |
| 166 | "Rename Tab" (use title case for window titles) | |
| src/dolphintabwidget.cpp | ||
| 33 | Don't want this in production code | |
| src/dolphintabwidget.h | ||
| 176 | Unnecessary whitespace change | |
Comment Actions
Thanks for the review.
Note to self:
Also tab detaching, tab closing and check if other operations on tabs exist.
Comment Actions
I've updated the diff, fixing both the issue with tab movement and styling.
I don't know if it would be a good idea to add a parameter (--tabname or something) to main.cpp to allow passing the tab name after detachement. What do you think?
Comment Actions
Nah, I don't think that's needed because Dolphin doesn't let you change the window title in the first place.
| src/dolphintabbar.cpp | ||
|---|---|---|
| 23 | Unused, please remove it. | |
| 164 | We could use a more descriptive variable name here, for example renamed or similar. | |
| 165 | Please add const | |
| 166 | Maybe we can add a colon at the end of "New tab name" ? | |
| src/dolphintabpage.h | ||
| 143 | Coding style: please call it uuid(), we don't usually use the getXXX naming style for getters. | |
| src/dolphintabwidget.cpp | ||
| 38 | Not needed, the constructor is implicitly called for QMap objects. | |
| 279 | Coding style: brace should go to next line for functions. | |
| 285 | Coding style: else starts at the end of previous line. | |
| src/dolphintabwidget.cpp | ||
|---|---|---|
| 120 | Maybe this logic should be moved to the tabName function instead? | |
| src/dolphintabwidget.h | ||
| 226 | Why is this information stored here and not in the individual Tab Pages? If a Tab Page knew it's own name, you wouldn't need the UUID stuff so it could simplify the code quite a bit. | |
Comment Actions
@meven has this been moved to commandeered and moved to Gitlab? I think you can safely do so if you haven't as the original author never replied.