davidre (David Redondo)
User

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Friday

  • Clear sailing ahead.

User Details

User Since
Nov 21 2018, 9:30 AM (39 w, 6 h)
Availability
Available

Recent Activity

Today

davidre accepted D23162: Add option to additionally save screenshot to clipboard.
Wed, Aug 21, 2:05 PM · Spectacle
davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

All done. I hope the names a re better now. Any recommendation for kde-dev best-practices?

Wed, Aug 21, 2:04 PM · Spectacle
davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

Sorry for being annoying but I had a last look :)

Wed, Aug 21, 1:59 PM · Spectacle
davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

Almost good to go now :)

Wed, Aug 21, 1:45 PM · Spectacle
davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

Last nitpicks:

  • Since with your other patch this setting will control what happens after a Screenshot is taken and does not control what to copy to the clipboard, could you rename some variables accordingly?

is this renaming ok:
CopyImageToClipboardSetting -> AfterTakingScreenshotAction (everything, including enum)

enum CopyImageToClipboardSetting : int { DoNotChangeClipboard = 0, CopyImageToClipboard }
to:
enum AfterTakingScreenshotAction : int { NoClipboardChange = 0, CopyImageToClipboard }

nothingToCopy -> doNothing

CopyToClipboardGroup -> mAfterTakingScreenshotGroup

Yes I was thinking of something like that! NoClipboardChange should also probably be something like doNothing like the button.

  • Write to GeneralConfig instead of GuiConfig. I plan to move some settings around so the place where they are configured corresponds to the their group.

As far as I can see, nothing of the stuff I changed and added gets written to GuiConfig. Can you link to something?
NVM, found it :)

Wed, Aug 21, 1:34 PM · Spectacle
davidre committed R166:431dc4ddec3e: Update reviewboard to phabricator inside README (authored by davidre).
Update reviewboard to phabricator inside README
Wed, Aug 21, 1:30 PM
davidre requested review of D23316: [WIP] Port towards KConfig XT.
Wed, Aug 21, 1:08 PM · Spectacle
davidre added inline comments to D23162: Add option to additionally save screenshot to clipboard.
Wed, Aug 21, 12:46 PM · Spectacle
davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

Last nitpicks:

  • Since with your other patch this setting will control what happens after a Screenshot is taken and does not control what to copy to the clipboard, could you rename some variables accordingly?
  • Write to GeneralConfig instead of GuiConfig. I plan to move some settings around so the place where they are configured corresponds to the their group.
Wed, Aug 21, 12:45 PM · Spectacle
davidre abandoned D20180: [RFC] Make notifications responsibility of SpectacleCore.

Landing this would make D23162 harder and the architecture while here simpler worse for that case. I will do an alternative patch after it is landed

Wed, Aug 21, 12:28 PM · Spectacle
davidre added inline comments to D23162: Add option to additionally save screenshot to clipboard.
Wed, Aug 21, 12:11 PM · Spectacle
davidre added inline comments to D23162: Add option to additionally save screenshot to clipboard.
Wed, Aug 21, 10:14 AM · Spectacle
davidre added inline comments to D23162: Add option to additionally save screenshot to clipboard.
Wed, Aug 21, 10:08 AM · Spectacle

Yesterday

davidre updated the diff for D20180: [RFC] Make notifications responsibility of SpectacleCore.

Remove qDebug()

Tue, Aug 20, 12:28 PM · Spectacle
davidre added a comment to D20180: [RFC] Make notifications responsibility of SpectacleCore.

I would still like to land this even if we lose the image inside Klipper. It doesn't appear there everytime anyway and pasting still works as mentioned earlier.

Tue, Aug 20, 11:01 AM · Spectacle
davidre added inline comments to D23162: Add option to additionally save screenshot to clipboard.
Tue, Aug 20, 10:34 AM · Spectacle
davidre added a comment to T11418: DBus mode settings.

Yes something like that. Thinking about it it could also be added to the Shortcuts page because it defines what happens when a Shortcut is pressed?

Tue, Aug 20, 10:14 AM · Spectacle
davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

For the enum. You would define it inside SpectacleConfig and then save the respective value instead of a config entry for each option.

Tue, Aug 20, 10:09 AM · Spectacle
davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

In general I'm fine with the approach taken here if we would just add this one setting. However we want to add at least one more button to this group. I think using an enum here and having only a single config entry would be better. Compare the printkeyactionrunning group.

Tue, Aug 20, 7:38 AM · Spectacle

Mon, Aug 19

davidre added a comment to T11418: DBus mode settings.

Instead, could we have the DBus mode invocations read the current settings from Spectacle itself? The user can already configure whether to include the cursor via the main UI; it seems like it would be preferable to read this setting rather than adding a second one in the settings window to control the same thing.

Mon, Aug 19, 7:50 PM · Spectacle
davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

I don't think Copy file location to clipboard after saving belongs into the "After taking screenshot:" button group. It's confusing that it specifies two different points in time.

That one bugs me too.
To make more sense, Don't change the clipboard content and Copy image to clipboard should definitely go into the same button group.
Which means that Copy file location to clipboard after saving goes into a separate group.

There's one inconsistency tho, when a user activates both, we get the case 4) i mentioned up above. Which is very esoteric. See here:

1st put the screenshot in the clipboard, after it has been saved replace the clipboard content with the file location

This seems like a pretty esoteric use case. What would be the purpose? Can you envision why someone would want to do this. Also note that this isn't currently a feature that exists, so we're not debating taking it away from anyone, but rather proposing to add it in the first place. That makes the bar for adding it much higher. :)

One approach would be:
Two groups. First one called After taking screenshot: and containing Don't change the clipboard content and Copy image to clipboard.
Second group called After saving image: with a checkbox Copy file location to clipboard after saving and placed in Save, rather than General (as it used to be)

Take this patch also into consideration: https://phabricator.kde.org/D23210

Good idea.
With D23210 in mind however I would probably do

After taking  screenshot:  o Do Nothing
                           o Copy to clipboard
                           o Save Image

And have the other checkbox in the Save category as you said.

Mon, Aug 19, 7:56 AM · Spectacle
davidre created T11418: DBus mode settings.
Mon, Aug 19, 7:51 AM · Spectacle
davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

I don't think Copy file location to clipboard after saving belongs into the "After taking screenshot:" button group. It's confusing that it specifies two different points in time.

Mon, Aug 19, 7:32 AM · Spectacle

Thu, Aug 15

davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

Thanks very much for the patch!

For patches that change the UI, it's customary to add a screenshot to the Test Plan section -- or even better, a video depicting it being used! Also the summary section is a good place to explain what your patch does, what problems it's solving, why you did it the way you did, etc.

In terms of behavior, maybe it would make sense to re-work the config UI so that each option has a radio button, and then only one can be active at once. Also, While this feature is on, I wonder if it makes sense to disable the Copy to Clipboard button on the main UI, since the screenshot will always be copied automatically.

+1 to radio buttons.
I would not disable the button since you could always want to copy the screenshot again after copying something else

Thu, Aug 15, 5:49 AM · Spectacle
davidre added a comment to D23162: Add option to additionally save screenshot to clipboard.

Thanks! I also thought about adding such a functionality.

Thu, Aug 15, 5:44 AM · Spectacle

Mon, Aug 12

davidre added a comment to D23080: [GTK3 -> Chrome/ium] Tweak tab stylings.

IMO the ideal state of affairs would be to somehow emulate the colors of the standard Qt tab widget:

Not too contrasty, but not too subtle either.

Mon, Aug 12, 4:33 PM · Plasma

Sun, Aug 11

davidre committed R32:0f21717e73ec: Merge branch '5.4' (authored by davidre).
Merge branch '5.4'
Sun, Aug 11, 3:16 PM
davidre committed R32:8c8c7b4120bf: [Grepview] Use the correct icons There are expand-all and collapse-all icons so… (authored by davidre).
[Grepview] Use the correct icons There are expand-all and collapse-all icons so…
Sun, Aug 11, 3:13 PM
davidre committed R491:6068b0dc8b80: Draw empty arrowheads closed (authored by davidre).
Draw empty arrowheads closed
Sun, Aug 11, 2:00 PM
davidre closed D23093: Draw empty arrowheads closed.
Sun, Aug 11, 2:00 PM
davidre updated the test plan for D23093: Draw empty arrowheads closed.
Sun, Aug 11, 1:12 PM
davidre updated the test plan for D23093: Draw empty arrowheads closed.
Sun, Aug 11, 1:12 PM
davidre requested review of D23093: Draw empty arrowheads closed.
Sun, Aug 11, 1:11 PM

Fri, Aug 9

davidre added a comment to D23049: Add Kirigami ListSectionHeader component.

Should this set sectionDelegate: true and not expose this to the outside?

Fri, Aug 9, 3:49 PM · Kirigami

Wed, Aug 7

davidre committed R32:9854fc81b7a1: Merge branch '5.4' (authored by davidre).
Merge branch '5.4'
Wed, Aug 7, 7:39 PM
davidre committed R32:c702a84702b3: Fix registry path inside kdevelop-msvc.bat (authored by davidre).
Fix registry path inside kdevelop-msvc.bat
Wed, Aug 7, 7:33 PM
davidre added a comment to D10068: [New Mail Notifier] Set default action to open email.

Can we revisit this? If you are notified about multiple mails there is no way to get from the notification to KMail currently since no buttons are shown.

Wed, Aug 7, 7:56 AM · KDE PIM

Tue, Aug 6

davidre added a comment to D20180: [RFC] Make notifications responsibility of SpectacleCore.

Something to look out for https://phabricator.kde.org/D22684?vs=62444&id=63145

Tue, Aug 6, 5:16 AM · Spectacle

Mon, Aug 5

davidre committed R32:6d5605cb6bf5: Merge branch '5.4' (authored by davidre).
Merge branch '5.4'
Mon, Aug 5, 12:08 PM
davidre committed R32:91191b8766dc: [Documentation] Set size policy of providers combobox to AdjustToContents (authored by davidre).
[Documentation] Set size policy of providers combobox to AdjustToContents
Mon, Aug 5, 11:58 AM

Sat, Aug 3

davidre committed R108:52eba31ea23d: [kcmkwin/kwineffects] Rework the Effects KCM (authored by davidre).
[kcmkwin/kwineffects] Rework the Effects KCM
Sat, Aug 3, 7:43 AM
davidre closed D22830: [kcmkwin/kwineffects] Rework the Effects KCM.
Sat, Aug 3, 7:43 AM · VDG, KWin

Thu, Aug 1

davidre updated the diff for D22830: [kcmkwin/kwineffects] Rework the Effects KCM.

restore me as the author

Thu, Aug 1, 6:59 PM · VDG, KWin
davidre updated the diff for D22830: [kcmkwin/kwineffects] Rework the Effects KCM.

That should work

Thu, Aug 1, 6:50 PM · VDG, KWin
davidre updated the test plan for D22830: [kcmkwin/kwineffects] Rework the Effects KCM.
Thu, Aug 1, 5:55 PM · VDG, KWin
davidre updated the diff for D22830: [kcmkwin/kwineffects] Rework the Effects KCM.

Arc mess up

Thu, Aug 1, 5:55 PM · VDG, KWin
davidre updated the diff for D22830: [kcmkwin/kwineffects] Rework the Effects KCM.

Rework

  • Use virtual desktops kcm list style and Kirigami actions
  • Remove info button. The copyright info can be accessed by clicking on the item
Thu, Aug 1, 5:54 PM · VDG, KWin
davidre committed R296:e310144eb8e9: Scrollview - Don't fill the parent with the view (authored by davidre).
Scrollview - Don't fill the parent with the view
Thu, Aug 1, 5:35 PM
davidre closed D22827: Scrollview - Don't fill the parent with the view.
Thu, Aug 1, 5:35 PM · Frameworks
davidre committed R256:e2cb1f8efc38: Make spectacle work (authored by davidre).
Make spectacle work
Thu, Aug 1, 11:45 AM
davidre closed D22870: Make spectacle work.
Thu, Aug 1, 11:45 AM
davidre requested review of D22870: Make spectacle work.
Thu, Aug 1, 10:22 AM
davidre added a comment to T10968: Make kde.org/applications prettier.

Maybe make a app pages with homepages style like in Konsole? Where everything will be written about this programs.

Boring:


Kool:

We have many apps and not all of them have the manpower to do this. The kde.org/applications are autogenerated. If all apps had pages with manually written content many of them would be quickly out of date again. See also the list in T10827.
Because of this the current approach is better. Autogenerated sites for all apps and for the apps that have their own website and the manpower to maintain them we link to it.

Thu, Aug 1, 9:37 AM · Website Developers, KDE Applications
davidre added a comment to D22827: Scrollview - Don't fill the parent with the view.

So the alternative approach would be something like this:

diff --git a/src/qmlcontrols/kcmcontrols/qml/ScrollView.qml b/src/qmlcontrols/kcmcontrols/qml/ScrollView.qml
index 8ef57a2..443f063 100644
--- a/src/qmlcontrols/kcmcontrols/qml/ScrollView.qml
+++ b/src/qmlcontrols/kcmcontrols/qml/ScrollView.qml
@@ -48,14 +48,28 @@ QtControls.ScrollView {
     onViewChanged: {
         view.parent = scroll;
         view.anchors.fill = view.parent;
+        setMargins();
     }
Thu, Aug 1, 8:36 AM · Frameworks

Wed, Jul 31

davidre added a comment to D22844: [kcm-colors] Export colorscheme to GTK color definitions.

Sorry my bad, I misread the diff. I thought you added a new flag (as proposed above) and looked at exportGtkTheme but that flag is an existing one. And to my shame it doesn't even have a green background.

Wed, Jul 31, 2:02 PM · Plasma
davidre added a comment to D22844: [kcm-colors] Export colorscheme to GTK color definitions.

It seems the header with the new flag is missing from the diff.

Wed, Jul 31, 1:55 PM · Plasma

Tue, Jul 30

davidre added a comment to D22836: Fix checking dirs for metainfo.yaml with non-ascii chars with Python 2.7.

@kossebau also wrote

Tue, Jul 30, 9:23 PM · Documentation, Frameworks
davidre updated the summary of D22827: Scrollview - Don't fill the parent with the view.
Tue, Jul 30, 6:29 PM · Frameworks
davidre updated the summary of D22830: [kcmkwin/kwineffects] Rework the Effects KCM.
Tue, Jul 30, 6:28 PM · VDG, KWin
davidre added a reviewer for D22830: [kcmkwin/kwineffects] Rework the Effects KCM: VDG.
Tue, Jul 30, 2:28 PM · VDG, KWin
davidre added a reviewer for D22830: [kcmkwin/kwineffects] Rework the Effects KCM: KWin.
Tue, Jul 30, 2:24 PM · VDG, KWin
davidre requested review of D22830: [kcmkwin/kwineffects] Rework the Effects KCM.
Tue, Jul 30, 2:23 PM · VDG, KWin
davidre added a comment to D22827: Scrollview - Don't fill the parent with the view.

An alternative approach could be to set the respective anchor.*margin to view.parent.*padding.

Tue, Jul 30, 12:57 PM · Frameworks
davidre updated the test plan for D22827: Scrollview - Don't fill the parent with the view.
Tue, Jul 30, 12:55 PM · Frameworks
davidre added a reviewer for D22827: Scrollview - Don't fill the parent with the view: mart.
Tue, Jul 30, 12:55 PM · Frameworks
davidre updated the diff for D22827: Scrollview - Don't fill the parent with the view.

Commit message

Tue, Jul 30, 12:52 PM · Frameworks
davidre requested review of D22827: Scrollview - Don't fill the parent with the view.
Tue, Jul 30, 12:51 PM · Frameworks

Thu, Jul 25

davidre committed R120:ea32a7611227: [Image Wallpaper Slideshow] Allow setting of different sorting orders (authored by davidre).
[Image Wallpaper Slideshow] Allow setting of different sorting orders
Thu, Jul 25, 10:06 AM
davidre closed D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders.
Thu, Jul 25, 10:06 AM · Plasma
davidre updated the diff for D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders.

Last touches

  • remove QDebug
  • don't restore image if we are in random mode
  • if we have seen all images generate a new random order
Thu, Jul 25, 10:05 AM · Plasma

Wed, Jul 24

davidre accepted D22696: Use QTEST_GUILESS_MAIN.

Thanks

Wed, Jul 24, 9:10 AM · Spectacle

Jul 22 2019

davidre committed R247:ce1b8de5420d: Use the proper name for Spectacle (authored by davidre).
Use the proper name for Spectacle
Jul 22 2019, 10:08 AM
davidre closed D22639: Use the proper name for Spectacle.
Jul 22 2019, 10:08 AM · Sysadmin
davidre updated the diff for D22639: Use the proper name for Spectacle.

Also update description

Jul 22 2019, 9:59 AM · Sysadmin
davidre added a comment to D22639: Use the proper name for Spectacle.

I don't know, I just saw that the name was wrong.
But maybe we could just use "Screenshot capture utility" as does the desktop file.

Jul 22 2019, 9:56 AM · Sysadmin
davidre added a project to D22639: Use the proper name for Spectacle: Sysadmin.
Jul 22 2019, 9:49 AM · Sysadmin
davidre requested review of D22639: Use the proper name for Spectacle.
Jul 22 2019, 9:43 AM · Sysadmin

Jul 20 2019

davidre committed R166:1c92fa091384: [RFC] Also animate the Cancel button (authored by davidre).
[RFC] Also animate the Cancel button
Jul 20 2019, 8:11 AM
davidre closed D22324: [RFC] Also animate the Cancel button.
Jul 20 2019, 8:11 AM · Spectacle

Jul 19 2019

davidre added a comment to D22324: [RFC] Also animate the Cancel button.

Would love to have more feedback on this

Jul 19 2019, 5:05 PM · Spectacle
davidre updated subscribers of D20180: [RFC] Make notifications responsibility of SpectacleCore.

Ping? Maybe @davidedmundson has insight why it is only added to the history if the app quits

Jul 19 2019, 5:05 PM · Spectacle
davidre added a comment to D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders.

How can we move this forward? Are there things I overlooked? Is it sill crashing sometimes? Code comments?

Jul 19 2019, 5:04 PM · Plasma

Jul 16 2019

davidre committed R166:49f0e962f01b: Merge branch 'Applications/19.08' (authored by davidre).
Merge branch 'Applications/19.08'
Jul 16 2019, 7:52 PM
davidre committed R166:da30d66cc7fd: Change the screenshot button to cancel during the timeout (authored by davidre).
Change the screenshot button to cancel during the timeout
Jul 16 2019, 7:51 PM
davidre closed D22234: Change the screenshot button to cancel during the timeout.
Jul 16 2019, 7:51 PM · Spectacle
davidre added a comment to D22234: Change the screenshot button to cancel during the timeout.

String freeze is in two days if I'm reading https://community.kde.org/Schedules/Applications/19.08_Release_Schedule correctly.

Jul 16 2019, 7:34 AM · Spectacle
davidre added a comment to D22234: Change the screenshot button to cancel during the timeout.

Can we get this into 19.08?

Jul 16 2019, 7:33 AM · Spectacle

Jul 15 2019

davidre added a comment to T11124: Unify highlight effect style.

Thanks, can you give me an example of an application where I can find this behavior? I can't find anything in the applications I have installed.

Some of the Buttons in Spectacle are Toolbuttons instead of Pushbuttons. Namely "Save" and "Copy to Clipboard"

Jul 15 2019, 8:31 AM · VDG

Jul 13 2019

davidre updated the task description for T11221: Grepview - Group matches by line.
Jul 13 2019, 7:06 PM · KDevelop
davidre created T11221: Grepview - Group matches by line.
Jul 13 2019, 7:04 PM · KDevelop

Jul 12 2019

davidre added a comment to D20180: [RFC] Make notifications responsibility of SpectacleCore.

Yes you're right, I think I retestet the wrong version. However pasting still works.
It seems related to shortly quitting after copying. If I add
QTimer::singleShot(250, &QApplication::quit); to imageExported it appears in the history, but then we can't show the notification.

Jul 12 2019, 2:02 PM · Spectacle
davidre updated the diff for D20180: [RFC] Make notifications responsibility of SpectacleCore.

Break line

Jul 12 2019, 1:12 PM · Spectacle
davidre added a comment to D20180: [RFC] Make notifications responsibility of SpectacleCore.

For me it is the same behavior as master:

  • If you copy and quit on copy is checked it shows up in the history
  • If you copy and then exit manually it doesn't show up in the history but you can still paste it (for example into KolourPaint)

Notice that you have to check "Avoid empty clipboard" in Klipper settings. I think we came to the conclusion that this is the same behavior as for text.

Jul 12 2019, 1:11 PM · Spectacle
davidre updated the diff for D22121: [Image Wallpaper Slideshow] Allow setting of different sorting orders.
  • Only restore wallpaper on startup
  • Guard against empty path

I couldn't understand why this was being called with an empty string so I just guard
against it at the call site. Maybe this also helps @ngrahams crash but I still don't know
where it is coming from.

Jul 12 2019, 12:48 PM · Plasma
davidre updated the diff for D20180: [RFC] Make notifications responsibility of SpectacleCore.

rebase
Now that we fixed Klipper this should work

Jul 12 2019, 12:15 PM · Spectacle

Jul 11 2019

davidre committed R32:33b0f4e6296a: Revert "Grepview - Introduce a new intermediate level corresponding to lines" (authored by davidre).
Revert "Grepview - Introduce a new intermediate level corresponding to lines"
Jul 11 2019, 10:04 AM
davidre added a reverting change for R32:75a222b64e26: Grepview - Introduce a new intermediate level corresponding to lines: R32:33b0f4e6296a: Revert "Grepview - Introduce a new intermediate level corresponding to lines".
Jul 11 2019, 10:04 AM
davidre closed D22148: Fix on click not working.
Jul 11 2019, 8:26 AM · Spectacle
davidre committed R166:4f1e7fa23023: Fix on click not working (authored by davidre).
Fix on click not working
Jul 11 2019, 8:26 AM
davidre updated the diff for D22148: Fix on click not working.
  • Also hide when onClick is checked
Jul 11 2019, 8:24 AM · Spectacle