Shizcow / dmenu-rs

A pixel perfect port of dmenu, rewritten in Rust with extensive plugin support
GNU General Public License v3.0
202 stars 9 forks source link

fix: stest: filename instead of file path #52

Closed maheshbansod closed 1 year ago

maheshbansod commented 1 year ago

I'm using the dmenu_run script (via dwm), and found that the commands in the dmenu were showing up as full path instead of just file names. I've made the stest output similar to the original stest as it was more convenient for me to see just the file names up there.
Feel free to reject the PR if this is not desired behaviour for this project. I'm unaware of the edge cases and hence used unwrap here, since it hasn't failed for me i'm assuming it should be fine (guess i'll do the error handling when something actually breaks :P).

Shizcow commented 1 year ago

I've made the stest output similar to the original stest

This is desired behavior, thank you for the fix. Any chance you could add a test to src/stest/tests/integration_tests.rs too?

Guess I'll do the error handling when something actually breaks :+1:

maheshbansod commented 1 year ago

...I didn't make test before pushing, turns out all tests are failing. :(

maheshbansod commented 1 year ago

I'll check what I can do tonight, and update the PR. I'm thinking of modifying all the scripts set_up_* to return only the file name. let me know if there's a better way to go about it.

maheshbansod commented 1 year ago

@Shizcow I checked the tests and quite a few of them are failing even after reverting my change (running make test on the root directory). Unsure what I can do to fix the tests that are failing. I will probably give it a go again later this week or this weekend. Please feel free to take over and do whatever's necessary (i'm unsure if i'll be able to figure it out yet if i get a chance to work on it).

Shizcow commented 1 year ago

You got it. I'll try to make some time this week to see what's going on with the failed tests. Thanks for the heads up.

On August 7, 2023 3:40:03 PM EDT, Mahesh Bansod @.***> wrote:

@Shizcow I checked the tests and quite a few of them are failing even after reverting my change (running make test on the root directory). Unsure what I can do to fix the tests that are failing. I will probably give it a go again later this week or this weekend. Please feel free to take over and do whatever's necessary (i'm unsure if i'll be able to figure it out yet if i get a chance to work on it).

-- Reply to this email directly or view it on GitHub: https://github.com/Shizcow/dmenu-rs/pull/52#issuecomment-1668472022 You are receiving this because you were mentioned.

Message ID: @.***>

Shizcow commented 1 year ago

After looking into this some more, are you sure that changing the output to file name is correct? From your PR:

$ echo "/dev/tty" | target/stest 
tty

And from original stest:

$ echo "/dev/tty" | stest
/dev/tty

$ pacman -Qs dmenu
local/dmenu 5.2-1
    Generic menu for X

And when I run dmenu_run with dmenu-rs, I see just file names, not full paths. This might be an issue on your end.

maheshbansod commented 1 year ago

You're right. Thanks. After reseting the local repo, the outputs of both the stest are identical which is weird. Unsure what must have caused this. Anyway, I'll close the PR, since there's no issue now. Thanks for taking the time to investigate it.