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

[BUG] line's info field cannot start with number #1201

Closed
etosan opened this issue Sep 24, 2020 · 7 comments
Closed

[BUG] line's info field cannot start with number #1201

etosan opened this issue Sep 24, 2020 · 7 comments
Labels

Comments

@etosan
Copy link

etosan commented Sep 24, 2020

Version

Version: 1.6.0

Configuration

Not needed

Launch Command

rofi -show frame-pick -modi "frame-pick:./frame-picker"

$ cat frame-picker
#!/bin/sh

echo re-exec >&2
echo   ROFI_RETV="${ROFI_RETV}" >&2
echo   ROFI_INFO="${ROFI_INFO}" >&2

windows="0x300db4
0x300db6
0x300db5
"

test -z "${windows}" && exit 0

[ ${#} -gt 0 ] && {
  [ ! -z "${ROFI_INFO}" ] && hc jumpto "${ROFI_INFO}" && exit 0
}

printf  "\00prompt\37Pick window\n"

printf  "${windows}\n" | while read WID
do
  # ... get vars

  #blows up  
  WIN_LINE="[${WIN_PID}]\t[${WIN_CLASS}]\t${WID}\t\"${WIN_NAME}\"\00info\37${WID}"

  # "works" but $ROFI_INFO is not eq $WID then and cannot be used in script without processing
  #WIN_LINE="[${WIN_PID}]\t[${WIN_CLASS}]\t${WID}\t\"${WIN_NAME}\"\00info\37XXX${WID}"
  echo "${WIN_LINE}"
done

Steps to reproduce

  • Make script which outputs lines with metadata key named info set to values that begin with number eg '0x1234', '1234', '3456' or '0123', etc.
  • Select any choice
  • Examine ROFI_INFO env variable passed to a script

What behaviour you see

  • ROFI_INFO env variable is empty, eg zero

What behaviour you expect to see

  • ROFI_INFO env variable should be set to numeric value encoded in info field of the row selected
@etosan etosan added the bug label Sep 24, 2020
@DaveDavenport
Copy link
Collaborator

This works fine for me, with the right escape sequence.
Your escape sequence looks wrong (when tested on my system), and \37 is not interpreted as a `\x1f.

echo -en "\x1f" 

gives the right byte.

echo -en "\37"  

gives me \37 as string

echo -en "\037"

is correct, but

echo -en "\0370x12345" 

not, as it parses \0370 as one entry.

@etosan
Copy link
Author

etosan commented Sep 25, 2020

Sorry, you are right, indeed the error is on my side.

I should have elaborated a bit more.

Due to various reasons I am using /bin/sh aka dash. That shell does not understand extended echo syntax you are using in your examples. Only way to send exact byte in that shell is to use octal notation. As such \037 octal, indeed is \x1f hexa. So that part is right.

Thank you for taking time testing this in bash! I now see where the error lies: in dash string parsing.

I thought octal sequences are parsed and expanded first, and only then variables are expanded in the dash string. What seem to be happening is the opposite instead: in "\037${WID}" dash first expands WID, and only then it goes to expand octals. Now this behavior makes sense to me, as variable themselves can contain byte octals and thus have to be expanded first. So, after first WID expansion string essentially becomes "\0370x12345" and that is bogus, as you already said, resulting in invalid octal character. The whole line then loses sense.

One way to "break" dash from octal parsing mode is to insert non numeric character before WID expansion, and then strip it from ROFI_INFO later (that's why the XXX version "works").

Would you be so kind and record this gotcha for posterity please? For example in manual under "other shells caveats" section (or something like that)?

@sardemff7
Copy link
Collaborator

FTR, you can use \0037 just fine, syntax is “\0 followed by one to three octal digits”, using leading extra 0 is valid and probably recommended for these cases. Maybe should you suggest that addition to dash man page?

@etosan
Copy link
Author

etosan commented Sep 27, 2020

Okay I tried you aporoach @sardemff7, and it works fine as well, so thank you for that! Dash man page addition should certainly be made, but wouldn't it be also wise to have at least warning in rofi docs - or is rofi policy "other shells than bash are unsupported, eg. you problem" ?

@DaveDavenport
Copy link
Collaborator

@etosan
Copy link
Author

etosan commented Sep 28, 2020

Thank you very much @DaveDavenport !

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants