D28619: BookmarksRunner: Remove caching of profile database path

[Alexander Lohnau]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:768e6a0a229d: BookmarksRunner: Remove caching of profile 
database path (authored by alex).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28619?vs=80580=81651

REVISION DETAIL
  https://phabricator.kde.org/D28619

AFFECTED FILES
  runners/bookmarks/browsers/firefox.cpp

To: alex, #plasma, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28619: BookmarksRunner: Remove caching of profile database path

[Alexander Lohnau]

alex updated this revision to Diff 80580.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28619?vs=80578=80580

REVISION DETAIL
  https://phabricator.kde.org/D28619

AFFECTED FILES
  runners/bookmarks/browsers/firefox.cpp

To: alex, #plasma, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28619: BookmarksRunner: Remove caching of profile database path

[Alexander Lohnau]

alex updated this revision to Diff 80578.
alex added a comment.


  Implement requested changes
  
  Thanks, I will do that after the bugfix is done :-)

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D28619?vs=79466=80578

BRANCH
  arcpatch-D28619

REVISION DETAIL
  https://phabricator.kde.org/D28619

AFFECTED FILES
  runners/bookmarks/browsers/firefox.cpp

To: alex, #plasma, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28619: BookmarksRunner: Remove caching of profile database path

[Stefan Brüns]

bruns requested changes to this revision.
bruns added a comment.
This revision now requires changes to proceed.


  Please just remove the `writeEntry`.
  
  You can then propose "But what about renaming the dbfile entry, maybe to 
alternate_dbfile?" as another review.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D28619

To: alex, #plasma, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28619: BookmarksRunner: Remove caching of profile database path

[Alexander Lohnau]

alex added a comment.


  Friendly ping :-)

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D28619

To: alex, #plasma, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28619: BookmarksRunner: Remove caching of profile database path

[Alexander Lohnau]

alex added a comment.


  > I would argue for just removing the `grp.writeEntry("dbfile", m_dbFile)` 
line. This keeps the current behavior for the second group, and still solves 
the problem for the first group when there is no config yet. Only users of the 
second group with an existing config have to do a one-time change to their 
config, e.g. resetting/removing the "dbfile" config value.
  
  But what about renaming the dbfile entry, maybe to `alternate_dbfile` ? Then 
the second group of users has to adapt to the changes and
  the first group can be sure that the default profile gets looked up correctly 
(in case the config already exists).

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D28619

To: alex, #plasma, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28619: BookmarksRunner: Remove caching of profile database path

[Stefan Brüns]

bruns added a comment.


  There are two possible ways of changing the caching, and each one will break 
behavior for one of two different groups of users:
  
  1. People who rely on the krunner following the default profile.
  2. People who rely on krunner using the dbfile config entry.
  
  I would argue for just removing the `grp.writeEntry("dbfile", m_dbFile)` 
line. This keeps the current behavior for the second group, and still solves 
the problem for the first group when there is no config yet. Only users of the 
second group with an existing config have to do a one-time change to their 
config, e.g. resetting/removing the "dbfile" config value.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D28619

To: alex, #plasma, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D28619: BookmarksRunner: Remove caching of profile database path

[Alexander Lohnau]

alex created this revision.
alex added reviewers: Plasma, bruns, broulik.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
alex requested review of this revision.

REVISION SUMMARY
  As requested in D28196  this patch 
removes the caching feature.
  This caching causes issues when the user has multiple profiles, because the 
cached value
  is not updated as long as the database exists.
  
  Depends on D28473 .

TEST PLAN
  Should compile, no changes to lookup logic made.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  bookmarksrunner_firefox_fix_remove_caching (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D28619

AFFECTED FILES
  runners/bookmarks/browsers/firefox.cpp

To: alex, #plasma, bruns, broulik
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart