hallas (David Hallas)
User

Projects

User does not belong to any projects.

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Saturday

  • Clear sailing ahead.

User Details

User Since
Jul 16 2018, 3:21 PM (61 w, 2 d)
Availability
Available

Recent Activity

Tue, Sep 17

hallas added inline comments to D19311: Add navigation history to forward/back buttons.
Tue, Sep 17, 11:13 AM · Dolphin
hallas committed R244:3d6847425df1: Fix KListOpenFilesJob unit test on Unix if lsof is not installed (authored by hallas).
Fix KListOpenFilesJob unit test on Unix if lsof is not installed
Tue, Sep 17, 4:21 AM
hallas closed D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed.
Tue, Sep 17, 4:19 AM · Frameworks

Sat, Sep 14

hallas added inline comments to D19311: Add navigation history to forward/back buttons.
Sat, Sep 14, 3:49 PM · Dolphin

Thu, Sep 12

hallas added inline comments to D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed.
Thu, Sep 12, 5:32 PM · Frameworks
hallas updated the diff for D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed.

Use QSKIP to skip tests

Thu, Sep 12, 5:32 PM · Frameworks

Wed, Sep 11

hallas added inline comments to D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed.
Wed, Sep 11, 5:57 PM · Frameworks
hallas updated the diff for D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed.

Use QStandardPaths::findExecutable to locate lsof

Wed, Sep 11, 5:57 PM · Frameworks
hallas requested review of D23884: Fix KListOpenFilesJob unit test on Unix if lsof is not installed.
Wed, Sep 11, 5:35 PM · Frameworks
hallas committed R244:e119ce7b5afe: Add KListOpenFilesJob (authored by hallas).
Add KListOpenFilesJob
Wed, Sep 11, 4:10 PM
hallas closed D21760: Add KListOpenFilesJob.
Wed, Sep 11, 4:10 PM · Frameworks
hallas updated the diff for D21760: Add KListOpenFilesJob.

Updated @since

Wed, Sep 11, 4:03 PM · Frameworks

Mon, Sep 9

hallas added a comment to D19311: Add navigation history to forward/back buttons.

Thanks @hallas. I'm good with this now. For the future, I would like one of the following UI improvements:

  • Show no visible arrow at all (i.e. what web browsers do)
  • Make the downward-pointing arrow be really tiny and sit in the bottom-right corner of a square toolbutton, rather than making the toolbutton wider to accommodate it
Mon, Sep 9, 5:19 PM · Dolphin

Sat, Sep 7

hallas added a comment to D19311: Add navigation history to forward/back buttons.

Thanks for the analysis :) I still think we should get the change in with the current KToolBarPopupAction functionality (like Nate suggest), and then work on refining the user experience. But feel free to pinch in on the user experience part.

This is such a big discussion already. Which suggestion of Nate do you refer to specifically? I thought the current approach that shows indicators was denied.

Sat, Sep 7, 11:00 AM · Dolphin
hallas updated the summary of D19311: Add navigation history to forward/back buttons.
Sat, Sep 7, 8:12 AM · Dolphin
hallas updated the diff for D19311: Add navigation history to forward/back buttons.

Rebased

Sat, Sep 7, 8:12 AM · Dolphin
hallas added a comment to D19311: Add navigation history to forward/back buttons.

We have previously discussed various ways to simplify this change, but no other suitable solutions has been found, but I am still open to simpler solutions :)

This is what I think can produce the wanted behaviour:
KToolBarPopupAction uses QToolButton internally so Indicators are drawn (because of style rules if I understand correctly). If there is no way to use QToolButton and hide indicators without forcing style changes (which was blocked) then it seems to me like the only way forward is not using a QToolButton.
AFAICT there are two ways forward:

  • Further modify KToolBarPopupAction to not use QToolButton internally in the case of menuIndicator being turned off and instead internally implement the same behaviour using some other button class. The QAbstractButton with it's pressed and clicked signals might be enough to handle it but there are possibly more fitting specialised button classes. This way no changes to the Breeze style or any other style for that matter should be necessary.
  • It might be a bit difficult to imitate the behaviour of KToolBarPopupAction without using a QToolButton so alternatively it might be easier to not modify KToolBarPopupAction at all and create or use a different class. In this case you don't have to mimic all options of KToolBarPopupAction perfectly since it is expected that the new class behaves differently. You could then copy from/use Falkon's NavigationBarToolButton (like @david.fontanals suggested in D19311#432597) . I'm pretty sure the license of that class is incompatible with frameworks though so that would have to be handled if it's supposed to land in KWidgetAddons.

    Take my analysis with a grain of salt though since it is mainly based on me reading documentation without having ever actually tried similar stuff.
Sat, Sep 7, 8:08 AM · Dolphin
hallas added a comment to D19311: Add navigation history to forward/back buttons.

Hmm, can you rebase it on master?

Sat, Sep 7, 8:07 AM · Dolphin
hallas added a comment to D21760: Add KListOpenFilesJob.

This is good to go in :-)

If you push it today it'll indeed be in 5.62, to be tagged tomorrow, otherwise we'll need to adjust the @since tag ;-)

Sat, Sep 7, 8:05 AM · Frameworks

Fri, Sep 6

hallas updated the summary of D21760: Add KListOpenFilesJob.
Fri, Sep 6, 10:24 AM · Frameworks
hallas added inline comments to D21760: Add KListOpenFilesJob.
Fri, Sep 6, 10:24 AM · Frameworks
hallas updated the diff for D21760: Add KListOpenFilesJob.

Review comments

Fri, Sep 6, 10:23 AM · Frameworks
hallas added a comment to D19311: Add navigation history to forward/back buttons.

At this point I'm inclined to say that we should go forward with the patch regardless of widget changes, because otherwise it's gonna be stuck in patch review limbo forever.

Can you make the menu show up on press-and-hold without showing any arrow on the buttons without widget changes? That's probably the simplest path forward until we sort out the appearance issue. Better to temporarily make it semi-hidden than never getting it landed. :)

Fri, Sep 6, 8:50 AM · Dolphin
hallas added a comment to D19311: Add navigation history to forward/back buttons.

Dolphin appears to not compile without F6773036 in KWidgetAddons though. All I can find for F6773036 is the diff. Is there a Dxxxxx thing in phab for that?

Fri, Sep 6, 4:48 AM · Dolphin
hallas added a comment to D19311: Add navigation history to forward/back buttons.

At this point I'm inclined to say that we should go forward with the patch regardless of widget changes, because otherwise it's gonna be stuck in patch review limbo forever.

Can you make the menu show up on press-and-hold without showing any arrow on the buttons without widget changes? That's probably the simplest path forward until we sort out the appearance issue. Better to temporarily make it semi-hidden than never getting it landed. :)

Fri, Sep 6, 4:48 AM · Dolphin
hallas updated the diff for D21760: Add KListOpenFilesJob.

Review comments, renamed the files to match the class name

Fri, Sep 6, 4:46 AM · Frameworks
hallas added a comment to D21760: Add KListOpenFilesJob.

Yes, the filenames should match the classname, obviously :)

I did a deeper review of the KJob usage and I have two more comments, sorry for not taking the time to do this earlier...

Fri, Sep 6, 4:46 AM · Frameworks

Thu, Sep 5

hallas added a comment to D19311: Add navigation history to forward/back buttons.

Looks good. I do have a weird occurrence with three slashes appearing in the path for

seems when I leave and reenter, then leave the home dir.

Thu, Sep 5, 5:20 AM · Dolphin

Tue, Sep 3

hallas added inline comments to D21760: Add KListOpenFilesJob.
Tue, Sep 3, 3:14 PM · Frameworks
hallas added a comment to D21760: Add KListOpenFilesJob.

Given that the namespace doesn't contain anything else anymore, I would just get rid of it, and provide a single class, KListOpenFilesJob.

Tue, Sep 3, 3:12 PM · Frameworks
hallas updated the diff for D21760: Add KListOpenFilesJob.

Removed KListOpenFiles namespace and renamed ListOpenFilesJob to KListOpenFilesJob

Tue, Sep 3, 3:12 PM · Frameworks
hallas updated the summary of D21760: Add KListOpenFilesJob.
Tue, Sep 3, 3:11 PM · Frameworks
hallas updated the summary of D21760: Add KListOpenFilesJob.
Tue, Sep 3, 5:29 AM · Frameworks
hallas added inline comments to D21760: Add KListOpenFilesJob.
Tue, Sep 3, 5:27 AM · Frameworks
hallas updated the diff for D21760: Add KListOpenFilesJob.

Review comments

Tue, Sep 3, 5:27 AM · Frameworks

Mon, Sep 2

hallas added a comment to D19311: Add navigation history to forward/back buttons.

I'd like to test this... ...will Oxygen need similar changes?

Mon, Sep 2, 1:26 PM · Dolphin
hallas added a comment to D21760: Add KListOpenFilesJob.

I have added a minimal Windows implementation which always emits an error, along with a unit test. Please review it thoroughly and then I think it is ready to land :)

Mon, Sep 2, 1:22 PM · Frameworks
hallas updated the diff for D21760: Add KListOpenFilesJob.

Review comments. Added minimal Windows implementation which basically always reports failure with the error code Unsupported.

Mon, Sep 2, 1:21 PM · Frameworks

Fri, Aug 30

hallas added a comment to D21760: Add KListOpenFilesJob.

One thing, when this is ready to land I will address the Windows support so that we do not get broken builds :)

Fri, Aug 30, 5:20 AM · Frameworks
hallas added a comment to D21760: Add KListOpenFilesJob.

@dfaure - Overall, what do you think about the approach of subclassing KJob? Did it turn out like you had thought? And is this the solution we should go with, or was one of the other solutions better?

Fri, Aug 30, 5:19 AM · Frameworks
hallas updated the diff for D21760: Add KListOpenFilesJob.

Review comments

Fri, Aug 30, 5:19 AM · Frameworks

Thu, Aug 29

hallas updated the test plan for D23205: [KProcessList] Optimize KProcessList::processInfo.
Thu, Aug 29, 5:55 AM · Frameworks
hallas updated the diff for D23205: [KProcessList] Optimize KProcessList::processInfo.

Add bug reference

Thu, Aug 29, 5:53 AM · Frameworks
hallas updated the diff for D23205: [KProcessList] Optimize KProcessList::processInfo.

Fixed review comments, rebased.

Thu, Aug 29, 5:52 AM · Frameworks
hallas added inline comments to D21760: Add KListOpenFilesJob.
Thu, Aug 29, 5:27 AM · Frameworks
hallas added a comment to D21760: Add KListOpenFilesJob.

@meven , so I finally managed to rewrite this patch to use KJob instead. Please take a look at it again and see if this is better approach :)

Thu, Aug 29, 5:25 AM · Frameworks
hallas updated the diff for D21760: Add KListOpenFilesJob.

Rewrote the code to use KJob

Thu, Aug 29, 5:24 AM · Frameworks

Aug 17 2019

hallas added inline comments to D21760: Add KListOpenFilesJob.
Aug 17 2019, 12:39 PM · Frameworks
hallas added a comment to D23214: Add a BlockinApp util class to find process blockng access.

Hey @meven I have been working on the same thing D21760 - maybe we should consolidate our efforts? The code you have written looks very similar to what I have been doing :) As you can read in the review comments for D21760 the current suggestion is to look into doing a KJob subclass.

Great suggestion I missed this prior diff.

Abandoned in favor of D21760 and adding a KJob for this use case

Aug 17 2019, 12:13 PM · Frameworks
hallas added a comment to D23214: Add a BlockinApp util class to find process blockng access.

Hey @meven I have been working on the same thing D21760 - maybe we should consolidate our efforts? The code you have written looks very similar to what I have been doing :) As you can read in the review comments for D21760 the current suggestion is to look into doing a KJob subclass.

Aug 17 2019, 12:02 PM · Frameworks

Aug 16 2019

hallas requested review of D23205: [KProcessList] Optimize KProcessList::processInfo.
Aug 16 2019, 4:28 PM · Frameworks

Aug 13 2019

hallas added inline comments to D21760: Add KListOpenFilesJob.
Aug 13 2019, 3:21 PM · Frameworks
hallas updated subscribers of D21760: Add KListOpenFilesJob.

You wrote "ported from Device Notifier". I haven't check in detail yet, but do other copyright holders need to be mentioned?

Aug 13 2019, 3:13 PM · Frameworks

Aug 8 2019

hallas added a comment to D19311: Add navigation history to forward/back buttons.

@ngraham - Hey Nate, if you have time I would like to pick this patch up again, what are your thoughts on how we progress this? Currently this patch depends on D20867 being merged and released and also a change to KWidgetAddons (F6773036), so one approach could be to focus the review effort on those changes first? We have previously discussed various ways to simplify this change, but no other suitable solutions has been found, but I am still open to simpler solutions :)

Aug 8 2019, 5:45 AM · Dolphin

Jun 27 2019

hallas added a comment to D21760: Add KListOpenFilesJob.

Ping :D

Jun 27 2019, 6:15 PM · Frameworks

Jun 26 2019

hallas added a comment to D20867: Add Property to disable drawing of menu arrow indicators.

Hi @ngraham - I have looked into this patch set and it is a little tedious to test, you have to do the following:

Jun 26 2019, 4:12 PM · Plasma
hallas updated the diff for D20867: Add Property to disable drawing of menu arrow indicators.

Rebased

Jun 26 2019, 4:05 PM · Plasma

Jun 21 2019

hallas added a comment to D20867: Add Property to disable drawing of menu arrow indicators.

Actually maybe it just needs a rebase on master...

Jun 21 2019, 4:52 AM · Plasma

Jun 20 2019

hallas added a comment to D19311: Add navigation history to forward/back buttons.

So what needs to happen here now? I'll admit I'm a bit lost. Do we need a Breeze patch, or can we set the correct hints here?

Jun 20 2019, 5:29 PM · Dolphin

Jun 16 2019

hallas requested review of D21849: Fixes crash when copying hotspot.
Jun 16 2019, 3:30 PM · Konsole

Jun 12 2019

hallas added a comment to D21760: Add KListOpenFilesJob.

First of, this is WIP, I just wanted to share this early to get some feedback. The reasoning for this is to generalize functionality for running lsof, this is currently in use by the Device Notifier applet and I would like to use it in Dolphin to fix bug #189302. Also see the discussion in D19989 for details.

Jun 12 2019, 9:29 AM · Frameworks
hallas requested review of D21760: Add KListOpenFilesJob.
Jun 12 2019, 9:24 AM · Frameworks

Jun 1 2019

hallas added inline comments to D21235: Add handling of fuseiso filesystem type.
Jun 1 2019, 6:26 AM · Frameworks

May 28 2019

hallas added inline comments to D21235: Add handling of fuseiso filesystem type.
May 28 2019, 4:57 AM · Frameworks

May 27 2019

hallas added inline comments to D21235: Add handling of fuseiso filesystem type.
May 27 2019, 5:41 AM · Frameworks

May 20 2019

hallas added a comment to D19989: Unmounting busy device doesn't tell who is blocking.

@davidedmundson - Hi David! I am (finally) starting to look at implementing the lsof functionality in Solid, but I am unsure of where exactly to place it in Solid? Also, currently Solid does not depend on any KF5 libraries, this would be the first, is that ok?

May 20 2019, 5:24 PM · Dolphin

May 15 2019

hallas added inline comments to D21235: Add handling of fuseiso filesystem type.
May 15 2019, 7:30 PM · Frameworks
hallas requested review of D21235: Add handling of fuseiso filesystem type.
May 15 2019, 7:29 PM · Frameworks

May 13 2019

hallas abandoned D20024: Fixes crash when hiding devices.

This has been fixed in D21050

May 13 2019, 3:00 PM · Dolphin
hallas committed R318:78540e49213e: Summary: Fixes crash when hiding devices (authored by hallas).
Summary: Fixes crash when hiding devices
May 13 2019, 2:58 PM
hallas closed D21050: Summary: Fixes crash when hiding devices.
May 13 2019, 2:58 PM · Dolphin
hallas added a comment to D20867: Add Property to disable drawing of menu arrow indicators.

Hi @ngraham - any update on this one? Should we move forward with this approach or should we do something else?

May 13 2019, 2:55 PM · Plasma
hallas committed R318:2fac50f5f59b: Add Bookmark Handling (authored by hallas).
Add Bookmark Handling
May 13 2019, 2:37 PM
hallas closed D19926: Add Bookmark Handling.
May 13 2019, 2:37 PM · Dolphin

May 6 2019

hallas committed R244:fdf02e26749d: Add GetProcessList for retrieving the list of currently active processes (authored by hallas).
Add GetProcessList for retrieving the list of currently active processes
May 6 2019, 5:48 PM
hallas closed D20007: Add GetProcessList for retrieving the list of currently active processes.
May 6 2019, 5:48 PM · Frameworks
hallas added a comment to D20007: Add GetProcessList for retrieving the list of currently active processes.

Thanks for the review! Landing it now.

May 6 2019, 5:48 PM · Frameworks
hallas added a comment to D20024: Fixes crash when hiding devices.

Please also update the commit message (the fix is now different).

May 6 2019, 5:35 PM · Dolphin
hallas requested review of D21050: Summary: Fixes crash when hiding devices.
May 6 2019, 5:34 PM · Dolphin
hallas added a comment to D19926: Add Bookmark Handling.

@elvisangelaccio - is it ok to merge this change now or do we need to wait for anything else?

May 6 2019, 5:28 PM · Dolphin
hallas added a comment to D20007: Add GetProcessList for retrieving the list of currently active processes.

@davidedmundson - ping ?

May 6 2019, 5:28 PM · Frameworks
hallas added a comment to D20867: Add Property to disable drawing of menu arrow indicators.
In D20867#461499, @mart wrote:

QToolButton::setArrowType?

I just tried that, but it doesn't seem like Breeze honors this property - at least not for the QToolButton produced by KToolBarPopupAction. So adding

That's probably a breeze bug and that's where it should be addressed.
an application can't link to breeze, especially if is private api. (and would explode dependencies in distros package managers)

May 6 2019, 5:27 PM · Plasma
hallas abandoned D20938: Add Mounts Backend.
May 6 2019, 5:18 PM · Frameworks
hallas added a comment to D20938: Add Mounts Backend.

This patch has been superseeded by the wotk @bruns has done in D20995 - so closing this one :D

May 6 2019, 5:18 PM · Frameworks
hallas added inline comments to D21041: [Fstab] Use folder-decrypted icon for encrypting fuse mounts.
May 6 2019, 5:14 PM · Frameworks
hallas added inline comments to D20995: [Fstab] Add support for non-network filesystems.
May 6 2019, 3:29 PM · Frameworks
hallas added inline comments to D20995: [Fstab] Add support for non-network filesystems.
May 6 2019, 3:10 PM · Frameworks

May 3 2019

hallas added a comment to D20938: Add Mounts Backend.

I have tried to modify the fstab backend to also show fuse mounts and a very simple prototype is this:

May 3 2019, 5:54 AM · Frameworks

May 2 2019

hallas added a comment to D20938: Add Mounts Backend.

Solid already has a working implementation for reading from /proc/mounts, the fstab backend.

Contrary to this code, the fstab backend does not poll every second using a timer, but correctly uses /proc/mounts changes notification via blocking read.

The only thing needed is a small extension to also hand out information for other filesystems than nfs and cifs.

This was exactly the kind of review comments I was looking for :D Let me take a new stab at this where I extend the current fstab with this functionality so we can see how that solution compares to this one.

I already have/had some code for this laying on my hard disk, I will post this as a WIP later today ...

May 2 2019, 5:41 PM · Frameworks
hallas added a comment to D20938: Add Mounts Backend.

Solid already has a working implementation for reading from /proc/mounts, the fstab backend.

Contrary to this code, the fstab backend does not poll every second using a timer, but correctly uses /proc/mounts changes notification via blocking read.

The only thing needed is a small extension to also hand out information for other filesystems than nfs and cifs.

May 2 2019, 5:39 PM · Frameworks
hallas added a comment to D20938: Add Mounts Backend.
In D20938#459149, @ivan wrote:

I'm torn between two approaches:

  • doing what you have done, maybe with a customization point - fusermount -u by default, something else for specific mount types;
  • disabling the teardown operation

    The rationale for the second one:
  • The users which create fuse mounts from the shell know how to unmount them;
  • Users which used a tool to mount something (like Plasma Vault) should use the same tool to unmount (these tools potentially do more than simple unmounting - like calling a destructor in C++ instead of just free :) );
  • Some applications will have their own mounts (for example, akonadi will use encrypted storage for storing indexes of encrypted mails) - you don't want to have those controlled by the user.*

    (*) we will also need to hide these from Places, but that is not important at this point.
May 2 2019, 5:38 PM · Frameworks

May 1 2019

hallas added a comment to D20938: Add Mounts Backend.
In D20938#459076, @ivan wrote:

Thanks for working on this.

I'd probably call this fusemounts backend as it handles only FUSE mounts and nothing else.

As for the teardown operation, some FUSE backends (cryfs >0.10) have their own unmount commands instead of fusermount -u.

May 1 2019, 5:23 PM · Frameworks
hallas added a comment to D20938: Add Mounts Backend.

This commit is still work-in-progress, but I would really like to get some feedback to the approach. Does it make sense to add a new backend? Or should this functionality be merged with one of the other backends (I was considering the fstab backend)?

May 1 2019, 2:26 PM · Frameworks
hallas requested review of D20938: Add Mounts Backend.
May 1 2019, 2:24 PM · Frameworks

Apr 29 2019

hallas updated the diff for D19926: Add Bookmark Handling.

Share bookmarks.xml with kfile

Apr 29 2019, 3:06 PM · Dolphin
hallas added a comment to D19926: Add Bookmark Handling.

We would probably like to allow the bookmarks to be accessible from KIOFileWidgets open/save dialog, somehow.
There is already a bookmark feature there, it would be great if the two matches.

KFileBookmarkHandler loads from "kfile/bookmarks.xml" the bookmarks.
Could this xml file be shared with dolphin ?

Apr 29 2019, 2:50 PM · Dolphin

Apr 28 2019

hallas added a comment to D20867: Add Property to disable drawing of menu arrow indicators.

QToolButton::setArrowType?

Apr 28 2019, 5:20 PM · Plasma
hallas added a comment to D20867: Add Property to disable drawing of menu arrow indicators.

QToolButton::setArrowType?

Apr 28 2019, 5:15 PM · Plasma
hallas added a comment to D20867: Add Property to disable drawing of menu arrow indicators.

There shouldn't be any Breeze specific code in client apps. It's a sign of something being wrong.

What's a navigation style toolbutton and where do you want to use this?

Apr 28 2019, 5:05 PM · Plasma
hallas added a comment to D19311: Add navigation history to forward/back buttons.

What happens if the user is using an old version of breeze that doesn't have this new property? Will they get the arrows?

Apr 28 2019, 10:51 AM · Dolphin