D14620: SymbolView: Add expand and parameter options to popup menu
noreply at phabricator.kde.org
Mon Aug 6 14:00:44 BST 2018
loh.tar updated this revision to Diff 39186.
loh.tar added a comment.
> I'm ready to accept this patch.
Wow, good news, thanks!
But now I'm curious which part of my text was so persuasive.
> Except for the actionTriggered() function
Woops! That start was not so nice. Next time please add a hint for more information below ;-)
> What I meant was that you could move this line to parseSymbols() and not add a new "wrapper" function with only one extra line of code...
I found a second line I have missed :-)
> ...If you absolutely want to save the one setEnabled() for the timer-based document update,
As said I want to save there the settings too. And that would then be unneeded often done.
Independently of that would it look not right to me that menu morph to do in parseSymbols().
> please use a more descriptive name like displayOptionChanged() or something similar.
With pleasure. Good suggestion!
> Examples of options that are not saved when changed from menus: View -> font size changes, View -> Word wrap -> *, View -> Border -> *, Tools -> Spelling -> *,....
Thanks for the list.
"Word wrap" and "Spelling" are both annoying. I have so often to fumble unneeded with that.
> It is a design decision to have permanent changes in the settings pages and temporary document specific changes in the menus.
Almost good, and right. But! ;-)
The wrong parts there are
- that these "permanent changes in the settings page" should only be "presets" when you choose an unseen document or create a new one (so far working) and
- that the changes done in the menu should be saved permanent in relation to the current document (can't give a test case, but I think it's not working or at least buggy)
That said, we have no "new document" in our context where we really need a preset. We have only fixed types of documents and I have doubts that it would make sense to view one C++ file with "this" and another C++ file with "that" settings.
So, how about to remove that page?
At least should there "Display functions parameters" be removed. It's only supported by three parser: cpp, php, ruby
Later I want remove/replace the fixed options m_macro, m_struct, m_func with some more flexible and fitting solution. I think I have made a FIXME note about this.
CHANGES SINCE LAST UPDATE
To: loh.tar, #kate, sars
Cc: sars, kwrite-devel, #kate, michaelh, kevinapavew, ngraham, demsking, cullmann, dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the KWrite-Devel