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

Add Interception builds of the Windows GUI app #1053

Merged
merged 5 commits into from
May 25, 2024

Conversation

eugenesvk
Copy link
Contributor

@eugenesvk eugenesvk commented May 19, 2024

Describe your changes. Use imperative present tense.

Add Interception builds of the Windows GUI app

(also based on the notification PR #1031 to make it easier to add later and also test since I can just observe mouse tooltips changing to see whether it works, so will rebase on whatever that one ends up being after review)

By the way, the docs say that

I think it has something to do with being installed in the privileged location of system32\drivers.

But on my system the install script didn't copy/move any of the dlls anywhere, so I guess that's the real reason why it's incosistent - if you copy the dlls to a PATH'ed location, it works, otherwise it fails
Are you sure that you have interception.dll in Windows\system32\drivers?

Fixes the other part of #990 (comment)

Checklist

  • Add documentation to docs/config.adoc
    • N/A
  • Add example and basic docs to cfg_samples/kanata.kbd
    • N/A
  • Update error messages
    • N/A
  • Added tests, or did manual testing
    • Yes

@eugenesvk
Copy link
Contributor Author

@gerhard-h could you please test these interception builds?

@eugenesvk eugenesvk force-pushed the gui_intercept_build branch 3 times, most recently from 32ec965 to fcacbf8 Compare May 20, 2024 08:41
@gerhard-h
Copy link
Contributor

@gerhard-h could you please test these interception builds?

the old compile error is still there

$ cargo build --features interception_driver --features cmd --features gui
   Compiling kanata v1.7.0-prerelease (C:\path\kanata)
error[E0061]: this function takes 2 arguments but 3 arguments were supplied
   --> src\main_lib\win_gui.rs:161:5
    |
161 |     Kanata::event_loop(kanata_arc, tx, ui)?;
    |     ^^^^^^^^^^^^^^^^^^               ----
    |                                      | |
    |                                      | unexpected argument of type `SystemTrayUi`
    |                                      help: remove the extra argument
    |
note: associated function defined here
   --> C:\path\kanata\src\kanata\windows\interception.rs:13:12
    |
13  |     pub fn event_loop(kanata: Arc<Mutex<Self>>, tx: Sender<KeyEvent>) -> Result<()> {
    |            ^^^^^^^^^^

For more information about this error, try `rustc --explain E0061`.
error: could not compile `kanata` (bin "kanata") due to 1 previous error

did something go wrong with the merge?
8730678 (HEAD -> main, origin/main, origin/HEAD, help) feat(layers): allow use of var for name (#1028)
520ab00 feat(chords): read chords from text file (#984)
c704b93 doc: adjust release template
d538d1c chore(win): kanata_passthru.dll to release just workflow (#1032)
06273d8 fix(win,gui): add a missing console disabling attribute (#1048)
34882a6 fix(win,gui): don't hide error message on GUI creation
490e2cc doc: update release template
e83c809 feat(win-gui): Add more correct winiov2 handling method to the gui app (#1047)

@eugenesvk
Copy link
Contributor Author

Did you check the latest-greatestd-rebased branch fcacbf8? Builds here just fine

did something go wrong with the merge?

No, I just rebase it on top of another PR to make it easier for me to test (and also to merge later), so it has those extra unrelated-to-interception commits, but it still works

@gerhard-h
Copy link
Contributor

Did you check the latest-greatestd-rebased branch fcacbf8? Builds here just fine

when I follow the link, there is a warning "This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository."

git cherry-pick fcacbf870ef24aae71e567bdbf06b5430f5908e2 or git show fcacbf870ef24aae71e567bdbf06b5430f5908e2
tell me fatal: bad object

even the base string like CFG, GUI_CFG_TX, GUI_ERR_MSG_TX, GUI_ERR_TX... not exists in win.rs for me

@eugenesvk
Copy link
Contributor Author

well, yeah, the commit isn't merged yet, so it indeed exists only in my fork eugenesvk@fcacbf8

Guess something's wrong with your local repo, it needs to be reset to this

@gerhard-h
Copy link
Contributor

Guess something's wrong with your local repo

I started over with git clone https://github.com/jtroo/kanata.git -> no compile

git clone https://github.com/eugenesvk/kanata.git -> no compile

finaly git checkout gui_intercept_build within your repo -> compiles and runs properly

…hutdown

otherwise it can't quit since the main GUI thread continues to be running
move interception event loop to a new thread to free up main thread for GUI
@jtroo jtroo merged commit 0cbae37 into jtroo:main May 25, 2024
4 checks passed
@jtroo
Copy link
Owner

jtroo commented May 25, 2024

Regarding:

I think it has something to do with being installed in the privileged location of system32\drivers.

It's been a while, but I do recall checking and seeing that the dll was indeed there but somehow wasn't being discovered correctly.

@eugenesvk eugenesvk deleted the gui_intercept_build branch May 26, 2024 11:10
@gerhard-h
Copy link
Contributor

works great :) but...

  • icon-match-layer-name yes does not work (icons .ico are in . \icon and \icons)
  • tooltip-layer-changes yes ;;|false| the default should be no not false (no even works )
  • notify-cfg-reload-silent yes silents lrdl but not when reloading per gui
  • (cmd) now runs visible (flashing) in foreground, wich is anoying

jtroo pushed a commit that referenced this pull request May 26, 2024
- Respect the silence if configured on gui reload via tray
- Fix `icon-match-layer-name` not matching by layer name

Fixes some of
#1053 (comment)
@eugenesvk
Copy link
Contributor Author

false is fine, it's a sample config, so shows some valid variety for options, the sound/icons are fixed in the linked PR

@eugenesvk
Copy link
Contributor Author

re (cmd) you're explicitly asking to launch command prompt, so isn't it supposed to... launch it?

I think you might need to find a separate Windows trick to hide the flashing window similar to something like https://whatsoftware.com/hidden-start-runs-batch-files-silently-without-flickering-console/ or https://superuser.com/questions/191149/how-to-execute-cmd-exe-silently etc from

As far as I know, there is no direct way with cmd to completely avoid a console window, you'd need to use an intermediary

@gerhard-h
Copy link
Contributor

As far as I know, there is no direct way with cmd to completely avoid a console window, you'd need to use an intermediary

The thing is: there was no flicker with the non gui build.
Also I use (cmd-output-keys timek.cmd) wich requires to read the output (and only the wanted output).
the straight forward (cmd-output-keys r#"start "" /MIN C:\path...\timedek.cmd"#) does not work

also
(cmd-output-keys powershell.exe -NoProfile -NoLogo -NonInteractive -Command r#"echo "(" ((Get-Date -Format "yyyy-MM-dd").toCharArray() -join " ") ")""#) is maximum explicit about not to flicker, but it does.

@eugenesvk
Copy link
Contributor Author

The thing is: there was no flicker with the non gui build

Because you had this whole non-flickering console. Console commands launched from within the console use existing console

is maximum explicit about not to flicker,

You could even be more diret -WindowStyle hidden :), though still woudn't help

start

this is a batch command, can only use it from cmd

The VBS method "works" - it hides the console window. But then you actually want to communicate output of a console command, and if you suppress console, you get no output! This seems conceptually incompatible, but maybe there is some hidden way to circumvent (feel free to google more deeply)

Think the only proper way something you've already discussed in other issues like #275:

@eugenesvk
Copy link
Contributor Author

By the way, why do you just use AutoHotkey? This seems to works without any flashes

(cmd "path2/AutoHotkey.exe" "path2/insert_date.ahk")

#Requires AutoHotKey 2
#NoTrayIcon
curtime := FormatTime(,"dddd MMMM d, yyyy H:mm:ss")
Send(curtime)

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.

None yet

3 participants