-
Notifications
You must be signed in to change notification settings - Fork 12k
Process escape sequences given in prompts #1173
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
Conversation
1decdf5
to
d85875f
Compare
d85875f
to
3c249a4
Compare
I don't understand why we need this? You could have put the actual characters in the string. Now you have a list of back slash escaped sequence that users may or may not aware. Now what if you want to put a json string to the prompt? |
Sorry, I must be overlooking something. Could you give a more concrete example of what this broke?
The example I gave before still works just the same. It injects the actual characters into the string and works just fine.
But now Windows users can do the same thing. I think this is much more likely than someone has a prompt they want to use that happens to have You mention json, but you don't have to escape your quotes. I just tried these and they worked:
It doesn't even silently eat the escape sequences when they don't mean anything. You can even still do this:
and it will use exactly that string. I guess maybe if your json text itself had an escape sequence in it then that could be interpreted as an actual newline, but surely that has to be a really uncommon use case for main? Still, if this is a big issue, we could always make a |
In powershell you can do |
By merging this: do we want to support this feature in future if it won't work or will break someone's prompt? |
There seems to be ways to do it in most shells, except Windows Command Prompt or in bat files. But not everyone knows how and it's different everywhere. I guess the other answer to the question "why?" was just for convenience. I think some people prefer to just have escape sequences and I have yet to see a prompt this breaks. I checked our example prompts and our example scripts and they all work. The tweet example provided by vanstepanovftw still works. I don't know why you think you have to escape your quotes, this doesn't change that. Like I said, I don't have a problem making a The json example works and the tweet example works. Edit: He deleted the tweet example but it just had single quotes in it. I don't think this does what people think it does. |
I think there's confusion over what this patch does. It just lets you use escape sequences like |
First, any Bourne compatible shell would allow you to do ./main -m models/7B/ggml-model.bin -n -1 --color -r "User:" --in-prefix " " --prompt \
'User: Hi
AI: Hello. I am an AI chatbot. Would you like to talk?
User: Sure!
AI: What would you like to talk about?
User:' without the modification introduce here. It's much clearer and simpler. Second, the chosen escape sequence introduced in this PR is so arbitrary, switch (input[++i]) {
case 'n': output.push_back('\n'); break;
case 't': output.push_back('\t'); break;
case '\'': output.push_back('\''); break;
case '\"': output.push_back('\"'); break;
case '\\': output.push_back('\\'); break;
default: output.push_back('\\');
output.push_back(input[i]); break; Do you expect user to specifically remember the prompt is parsed with these five and only five escape sequences? It's different from C. It's different from JSON. It's different from Bourne Shell's builtin echo. It's different from Bash's Third, it breaks the input/output parity, and breaks the compatibility between </dev/null ./main -r "User:" -p "$(cat my-previous-chat-with-bob.txt; echo " Think again?" ; echo "Bob:")" without double check if we have one of the escape sequence in the file. Wait, what are the escape sequence again? |
One more rant of the choice of the escape sequence (I promise this is the last one). ./main -p "Running under bash, can you tell how many backslashes we are giving to the model: \\\"" |
Programs that primarily deal with text will often escape sequences automatically: I also agree that escape sequences in general can obfuscate things. In your example, even without I'd also argue the input output parity is still there because you could use I'll make a pull request where escapes are only enabled with the use of |
In Linux we can use a bashism to inject newlines into the prompt:
But in Windows there's simply no way. This patch processes the escape sequence given in prompts.
In Linux and on Macs, it allows us a cleaner way of doing it:
And makes the same thing possible in Windows: