Enable three more plugins by default
ClosedPublic

Authored by gregormi on Nov 3 2018, 5:25 PM.

Details

Summary
  • Cuttlefish Icon Chooser
  • File Browser (not on Windows because it is unstable there)
  • Terminal Tool View (not on Windows because it does not work there)
Test Plan

Remove ~/.config and started Kate. All new plugins are enabled by default.

I also tested with a plugin that does exist (to simulate users that don't have the plasma5-sdk with the cuttlefish plugin installed). There was no error.

Diff Detail

Repository
R40 Kate
Branch
my_defaultsettings
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 4826
Build 4844: arc lint + arc unit
gregormi created this revision.Nov 3 2018, 5:25 PM
Restricted Application added a project: Kate. · View Herald TranscriptNov 3 2018, 5:25 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald Transcript
gregormi requested review of this revision.Nov 3 2018, 5:25 PM
gregormi abandoned this revision.Nov 3 2018, 5:25 PM
This comment was removed by gregormi.
gregormi updated this revision to Diff 44787.Nov 3 2018, 5:33 PM
This comment was removed by gregormi.
gregormi retitled this revision from Code clean: sort inserts alphabetically and remove spaces to Enable four more plugins by default.Nov 3 2018, 5:41 PM
gregormi edited the summary of this revision. (Show Details)
gregormi edited the test plan for this revision. (Show Details)
gregormi added a reviewer: Kate.
gregormi edited the summary of this revision. (Show Details)Nov 3 2018, 5:44 PM
ngraham added a subscriber: ngraham.Nov 3 2018, 5:44 PM

Phabricator is not great about multi-commit chains and really wants to squash everything. The sanest way to do it is to make two separate patches; one for each commit you want to end up in the repo. See https://community.kde.org/Infrastructure/Phabricator#If_the_patches_are_all_for_the_same_project

Either way, +1 from me. These are all very sensible plugins to have on by default.

sars added a subscriber: sars.Nov 3 2018, 8:40 PM

Hmm... File Browser works badly on Windows and Terminal Tool View not at all.

The preview plugin has really great potential, but is it ready for being enabled by default already?

I can't comment on the cuttlefishplugin that much as I have not used it before and now when I installed it today I don't know how to invoke it...

I think it is better to let people enable plugins as they needed them in stead of enabling too many by default.

In D16641#353621, @sars wrote:

I think it is better to let people enable plugins as they needed them in stead of enabling too many by default.

The problem with this approach is that it requires that they:

  1. Know about Kate's plugins
  2. Know that there are plugins that are disabled by default
  3. Know what the disabled plugins are or can do and understand the description

If it were up to me I'd enable them all by default. :p

I am with Kare here: I think we should care to not enable plugins by default that are known to be problematic on windows.
Can you #ifdef this or at least add a runtime check depending on the OS?

With repsect to the icon plugin: I have enabled it here, but I don't see anything?! :-) Any hints?

kossebau added a subscriber: kossebau.EditedNov 4 2018, 6:23 PM

About Document Preview: being the one who wrote and pushed that plugin, I meanwhile am a bit disillusioned about the technological approach taken there, and have been rather tempted to propose the removal of that plugin again, hoping for a future implementation on more solid ground.

While doing the documents preview plugin with KParts technology initially seemed a clever and smart idea to reuse existing code and technology, sadly kxmlgui is not prepared for the rivaling request for keyboard shortcuts both by the currently focused texteditor view as well as the requests for the same by the kparts plugin used for the type to be previewed. Which actually is an issue with toolviews with richer UI as well, but the KParts plugins living in the preview plugin just escalate this more, given they usually come with a set of actions and their shortcuts.

I guess I should simply upload a patch to remove that plugin again separately, for a focussed discussion instead of hijacking things here. Edit: Done, see D16668

I am with Kare here: I think we should care to not enable plugins by default that are known to be problematic on windows.
Can you #ifdef this or at least add a runtime check depending on the OS?

I'll try to find out how to do the ifdef to exclude Windows from the plugins that don't work there properly.

With repsect to the icon plugin: I have enabled it here, but I don't see anything?! :-) Any hints?

Right click in the editor text area. In the context menu, there is a new entry "Insert Icon with Cuttlefish".

gregormi updated this revision to Diff 45285.Nov 11 2018, 2:30 PM
  • Don't add katefilebrowserplugin and katekonsoleplugin by default on Windows
  • Don't add ktexteditorpreviewplugin by default because it should be replaced by something better
  • add some comments
gregormi edited the summary of this revision. (Show Details)Nov 11 2018, 2:33 PM
gregormi retitled this revision from Enable four more plugins by default to Enable three more plugins by default.

Ready from my side. Waiting for "ready to land"...

dhaumann accepted this revision.Nov 20 2018, 9:25 PM

I would have preferred to only have one #indef WIN32. Preprocess statements should be reduced to the minimum, imho.

This revision is now accepted and ready to land.Nov 20 2018, 9:25 PM
gregormi updated this revision to Diff 45922.EditedNov 20 2018, 10:32 PM
gregormi edited the summary of this revision. (Show Details)
  • Reduce amount of #ifdef
This revision was automatically updated to reflect the committed changes.