Using static object result in showing error message in one versioned tab to be shown in all versioned
Details
Details
- Reviewers
elvisangelaccio dfaure - Group Reviewers
Dolphin - Commits
- R318:9a9fbae98814: [versioncontrolobserver] Do not use static plugin objects
Diff Detail
Diff Detail
- Repository
- R318 Dolphin
- Lint
Lint Skipped - Unit
Unit Tests Skipped
| src/views/versioncontrol/versioncontrolobserver.cpp | ||
|---|---|---|
| 270 | This looks like an unrelated change. If a parent here is necessary, please add it with another commit. If instead this change is indeed related to this patch, please explain why in the commit message. I'm not confident about this code because there is multi-threading involved. | |
| src/views/versioncontrol/versioncontrolobserver.cpp | ||
|---|---|---|
| 270 | It prevents leak, in previous commit plugins leave to application lifetime, now not. | |
Comment Actions
I want to have this in 18.12.1, i really not prefer to have custom changes against upstream.
Comment Actions
Apparently it has a problem, if 2 or more versioned tabs are open (in distinct git directories) switching between does not update status/actions. It looks like in dolphin-plugin is used static object as well. So after all we don't want error in one tab to be multiplied to all opened, i've seen it often, since i'm using plugin daily, so easy fix is to update directory on tab activated, since observer has only private slots i'm using old syntax did you have better idea?
| src/views/dolphinview.cpp | ||
|---|---|---|
| 191 | @elvisangelaccio are you ok with this line? | |
Comment Actions
Sorry but I can't reproduce the bug you put in the commit message.
I tried to do a git checkout on a dirty repo, but the error message only shows up in the versioned tab (not in all tabs).
Please update the commit message with a proper test plan.
Comment Actions
Ok, make second tab versioned too, now error from first tab is shown in second as well.
Comment Actions
@alexeymin, can you try this patch as well, it's pretty annoying to have 5+ versioned tabs, error in one to be shown in others.
Comment Actions
I can reproduce an issue. Without patch, VCS error generated in one tab is displayed in all versioned tabs (but not in normal unversioned tabs). Also strangely error is multiplied only on the first occurence, if after first appearance of error I close all error messages in all tabs, and trigger error again, it only appears in current tab, and not in any other.
With this patch I cannot reproduce error multiplication among any tabs.
| src/views/dolphinview.cpp | ||
|---|---|---|
| 191 | Not really. What's this connection needed for? If we really need it, we should find a better way. | |
| src/views/dolphinview.cpp | ||
|---|---|---|
| 191 | Ensures current tab is in sync with gitWrapper in dolphin plugins witch is singleton i.e. when you click tab it call git status on current directory. This line is pretty simple fix, without adding other slot, if we want new syntax we should add new public slot that call internally delayedDirectoryVerification | |
| src/views/dolphinview.cpp | ||
|---|---|---|
| 191 | Ok, so it's another bugfix but unrelated from this patch. Please move it to another commit then. And yes, we should try to use the new syntax if possible. | |
Comment Actions
@anthonyfieroni Next time please add the summary of the revision to the actual commit message. You may want to setup arc to do that.