Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes "unknown" being returned when some apps open #64

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

michaeljelly
Copy link

I've been developing an electron application, and noticed that ActivityWatch regularly logged the window as unknown. From research, it seems that some applications aren't "scriptable" according to JXA/applescript, and so you can't get their attributes or properties, which was leading the original script to fail at executing both when setting mainWindow and later when setting title. This meant that everything for that window was recorded as unknown.

To fix this, I first try to get the window the old way, and if this fails then to get the name of the frontmost window as the title. This works very well, it seems to get appName and title accurately, and avoids failing to execute and getting Unknown app names.

Hope this helps!

I've been developing an electron application, and noticed that ActivityWatch regularly logged the window as unknown. From research, it seems that some applications aren't "scriptable" according to JXA/applescript, and so you can't get their attributes or properties, which was leading the original script to fail at executing both when setting mainWindow and later when setting title. This meant that everything for that window was recorded as unknown.

To fix this, I first try to get the window the old way, and if this fails then to get the name of the frontmost window as the title. This works very well, it seems to get appName and title accurately, and avoids failing to execute and getting Unknown app names.
ErikBjare and others added 2 commits February 8, 2022 17:00
On Mac, you can have an application open without a window.
This edge-case previously caused an error meaning time wasn't tracked.
I'm not 100% that a title of "No window" is the right approach, but it is the clearest description of what's happening I can see right now.
@michaeljelly
Copy link
Author

The code has become slightly uglier to account for Applications where there are no open windows. Let me know whether you prefer just one comment to explain the new try-catch, or a comment in each place.

url = Application(appName).documents[0].url();
title = Application(appName).documents[0].name();
title = Application(appName).documents[0].name();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please try to keep whitespace consistent (2 spaces for indentation, no end-of-line spaces)

// You can also have safari open with just a Devtools window, meaning no documents
// This edge-case previously caused an error meaning time wasn't tracked.
// I'm not 100% that a title of "No window" is the right approach, but it is the clearest description of what's happening I can see right now.
title = "No window"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a single comment is fine, but perhaps lift out the "No window" constant into a variable, and document the behavior by the declaration/first use?

@krlmlr
Copy link

krlmlr commented Sep 6, 2022

This solves the issues I'm having with RStudio on macOS. What's the best way to move forward?

@ErikBjare
Copy link
Member

This PR might not be needed if all goes well with the new default Swift-method #68, just released in v0.12.1

Thanks for your hard work on this @michaeljelly, but looks like there was different way that seems to work a lot better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants