Differential D17602

BuildView: Search item with needed data when navigate in error list
ClosedPublic
Actions

Authored by loh.tar on Dec 15 2018, 1:04 PM.

Details

Reviewers
sars
Group Reviewers
Kate
Commits
R40:33b3e153575f: BuildView: Search item with needed data when navigate in error list
Summary

When you click on some additional hint of an error happens without this
patch nothing, with this patch is jumped to the line in the source which
the error caused.

Furthermore move Prev/Next action now to next/prev error and not only next/prev item

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
loh.tar created this revision.Dec 15 2018, 1:04 PM
Restricted Application added a project: Kate. · View Herald TranscriptDec 15 2018, 1:04 PM
Restricted Application added a subscriber: kwrite-devel. · View Herald Transcript
loh.tar requested review of this revision.Dec 15 2018, 1:04 PM

I would like furthermore change:

  • The slots Prev/Next in a way that they not forward one item but one error. This way is the action labeled but perhaps is the current behaviour intended(?)
  • Add the buttons for Prev/Next which are missing.
  • Change displayModeSlider/displayModeLabel into a QCombobox, save some space.
  • Merge the buttons Build/Cancel so that they are morph by using a stacked widget. Unfortunately is Nate against such lovely stuff
sars added a comment.Dec 15 2018, 8:23 PM

I'm not sure why you have to move the setFocus() call to the beginning of the function... your comment says something about selection...

The style change is probably not needed here, tho I don't have much against fixing the style...

But otherwise it looks good.

sars added a comment.Dec 15 2018, 8:37 PM

Changing the next/previous slots to go to the items with line numbers would be a good change.

I don't think we need the buttons for next/previous. If you use the keyboard for navigating you want the shortcuts and if you use the mouse it is much quicker to just navigate the results and click the wanted item.

I'm not 100% happy with the slider either... I'm almost inclined to go back to the multiple tabs ("Target Settings", "Full output", "Parsed output", "Warnings & Errors", "Errors")

I don't think we are pressed on space to need to merge the buttons...

loh.tar added a comment.Dec 15 2018, 8:54 PM

I'm not sure why you have to move the setFocus() call to the beginning of the function... your comment says something about selection...

I move this up to reduce code redundancies. Without this patch was only the focus changed when we hit the right item, now happens this almost every time. But in these rare case would then the chosen item kept the focus, but that hurts literally my eyes.

I don't think we need the buttons for next/previous. If you use the keyboard for navigating you want the shortcuts and if you use the mouse it is much quicker to just navigate the results and click the wanted item.

I guess I didn't need these buttons too, but have the feeling they should be there

I don't think we are pressed on space to need to merge the buttons...

Well, I like such behaviour. So the space is not the reason :-)

Changing the next/previous slots to go to the items with line numbers would be a good change.

Good, will try to do it (here or extra patch?)

loh.tar updated this revision to Diff 47653.Dec 16 2018, 6:15 AM
loh.tar retitled this revision from BuildView: Search needed data when select an error in the list to BuildView: Search item with needed data when navigate in error list.
loh.tar edited the summary of this revision. (Show Details)
  • Search on Next/Prev an item which has the desired data
  • Modify comment about highlighting
  • Update Title/Summary to fit new added stuff

Was only a one-line change, so I decided to add it here

loh.tar added a subscriber: ngraham.Dec 16 2018, 6:45 AM
  • Even with LANG=C will some things not translated back due to the use of i18n as save key, IIRC
  • The German translations are BTW really terrible, but sadly they are kept in some other repository
  • Sometimes I miss the possibility to select/copy some lines of the output
  • Regarding "garish highlighting" (read that better?) I was about to disable the possibility to select an item by the UI file, but I guess you would dislike the idea. And, yes, it's then not so easy to see where we are

@ngraham It would be nice you could give a comment to our talk, most notably

  • Merge Build/Cancel buttons
  • Add Prev/Next buttons, as you see no buttons here but the actions are avaialble
  • Better solution to choose the view mode. I like the current more than the old "one tab per mode" but it's somehow strange

sars accepted this revision.Dec 16 2018, 4:58 PM
This revision is now accepted and ready to land.Dec 16 2018, 4:58 PM
Closed by commit R40:33b3e153575f: BuildView: Search item with needed data when navigate in error list (authored by loh.tar, committed by sars). · Explain WhyDec 16 2018, 5:01 PM
This revision was automatically updated to reflect the committed changes.
ngraham added a comment.Dec 16 2018, 7:59 PM

Sorry, no time to look right now. I'll accept whatever the Kate folks decide on.

Revision Contents
Changeset List

Diff 47653

View Options

addons/katebuild-plugin/plugin_katebuild.cpp

Loading...