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.

Sign up for GitHub

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

Jump to bottom

[Bug / Feature] stack for commands #219

Open
tsankuanglee opened this issue Aug 13, 2023 · 9 comments · May be fixed by #220
Open

[Bug / Feature] stack for commands #219

tsankuanglee opened this issue Aug 13, 2023 · 9 comments · May be fixed by #220
Assignees
@angelofallars @Shinyzenith @EdenQwQ

Comments

@tsankuanglee
Copy link
Contributor

tsankuanglee commented Aug 13, 2023 •
edited
Loading

Version Information:

  • Distribution Information ( run uname -a ): Linux 6.4.8-arch1-1
  • swhkd version ( swhkd -V ): 1.2.1

Describe the bug:

Commands chained by && seem to be executed together, disregarding mode stack changes. Example:

super + d
    @enter display

mode display swallow
b
    @escape && @enter display_selection && @enter brightness_selection && set_brightness.sh
endmode

mode display_selection swallow oneoff
{1,2,3}
    save_selection_to_file.sh {left,middle,right}
endmode

mode brightness_selection swallow oneoff
{1,2,3,4,5}
    save_brightness_to_file.sh {0,25,50,75,100}%
endmode

Expected behavior:
With @escape && @enter display_selection && @enter brightness_selection && set_brightness.sh, we expect when mode display_selection is popped, we continue to the next command @enter brightness_selection.

Actual behavior:
As soon as b fires @escape && @enter display_selection && @enter brightness_selection && set_brightness.sh, everything is executed right away, including the set_brightness.sh script, while we are still in the mode display_selection.

To Reproduce:
See example above.

Additional information:
Since we don't have the syntax like sxhkd as I mentioned in #67 , we can't exactly do a command with more than two expansions, e.g.

{1,2,3}; {b,c,t}; {1,2,3}
    display_setting.sh {left,middle,right} {brightness,contrast,temperature} {low,mid,high}

With the mode design, I can only get around it by temporarily saving a state for later use.

That said, mode actually provides a more clear and reusable design, if this bug/feature is addressed. For example, I can re-use the display_selection mode for not only brightness, but contrast, color temperature, etc., by entering that mode, and then continue with the next action.
@Shinyzenith
Copy link
Member

@EdenQwQ can you provide your input on this?

@EdenQwQ EdenQwQ linked a pull request Aug 14, 2023 that will close this issue
Open
@EdenQwQ
Copy link
Member

EdenQwQ commented Aug 14, 2023

@tsankuanglee Could you please check out #220 and let us know if the change meets your request

@tsankuanglee
Copy link
Contributor Author

tsankuanglee commented Aug 14, 2023 •
edited
Loading

@EdenQwQ , I have not got a the chance to test the code yet (will probably be a few days before I can find time to do that), but from the description of that PR, yes, it addresses exactly the problem I'm describing here. Thank you so much!

By the way, the example you gave in your PR is much more concise and clear!

@EdenQwQ
Copy link
Member

EdenQwQ commented Aug 14, 2023

@EdenQwQ , I have not got a the chance to test the code yet (will probably be a few days before I can find time to do that), but from the description of that PR, yes, it addresses exactly the problem I'm describing here. Thank you so much!

By the way, the example you gave in your PR is much more concise and clear!

Thanks! We won't merge it without your feedback, but take your time!

@tsankuanglee
Copy link
Contributor Author

Just an update that I'm still on this. I'm getting some weird rust index out of bounds errors:

thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 18446744073709551615', swhkd/src/daemon.rs:385:68

I'm also seeing this error with the release version, so I suspect it's something on my end. I'll resolve it and test your PR and report back.

@EdenQwQ
Copy link
Member

EdenQwQ commented Aug 23, 2023

Just an update that I'm still on this. I'm getting some weird rust index out of bounds errors:

thread 'main' panicked at 'index out of bounds: the len is 0 but the index is 18446744073709551615', swhkd/src/daemon.rs:385:68

I'm also seeing this error with the release version, so I suspect it's something on my end. I'll resolve it and test your PR and report back.

It's weird because I though I've solved this issue with the latest commit in that PR. Could you run git pull in command_stack_devel branch and confirm that you're synced with the branch? I'm not sure why this also happens with the release version, if that still happens please provide the method to trigger the bug. Sorry for the inconvenience.

@tsankuanglee tsankuanglee mentioned this issue Aug 24, 2023
Open
@tsankuanglee
Copy link
Contributor Author

tsankuanglee commented Aug 24, 2023 •
edited
Loading

@EdenQwQ thanks for the prompt. I created #222 for the index out of bound problem that happened on the release version.

To clarify, when I got the error with your PR (yes, I did pull the latest commit), I was using the correct config (as opposed to the incorrect one mentioned in #222), but with an extra @escape (which is semantically wrong, I know). I suspect we need to check for an empty stack before pop.

alt + ctrl + shift + super + w
    @enter web

mode web swallow oneoff
c
    @escape && touch /tmp/test
endmode

@tsankuanglee
Copy link
Contributor Author

tsankuanglee commented Aug 24, 2023 •
edited
Loading

@EdenQwQ By the way, here's an illegal config that I accidentally used (see the last line: no endmode). This same config causes different behavior for the release version and your latest PR commit:

  • your latest PR commit: weird behavior. I had to use another machine to kill the process to regain normal keyboard control.
  • release version: no error given. I did not lose keyboard control. I was able to use ctrl-c to terminate the process.

Not sure whether that's a useful data point for you to debug.

alt + ctrl + shift + super + w
    @enter web

mode web swallow oneoff
c
    @escape && touch /tmp/test
escape

@Shinyzenith
Copy link
Member

@EdenQwQ can you look into this dear?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@angelofallars angelofallars

@Shinyzenith Shinyzenith

@EdenQwQ EdenQwQ

Labels
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: added @pend internal command waycrate/swhkd
4 participants
@tsankuanglee @angelofallars @Shinyzenith @EdenQwQ