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

do not force the prompt file to end with a new line #908

Merged
merged 1 commit into from
Apr 13, 2023

Conversation

prusnak
Copy link
Collaborator

@prusnak prusnak commented Apr 12, 2023

No description provided.

j-f1
j-f1 previously approved these changes Apr 12, 2023
@slaren
Copy link
Collaborator

slaren commented Apr 12, 2023

The changes to .editorconfig look good, but I am not sure that it is a good idea to add a space to these prompts, since it can mess with the tokenization of the next word. Instead, we have --in-prefix " " for that purpose. reason-act.sh already has --in-prefix " ", and probably this should be added to chat.sh as well.

@j-f1 j-f1 dismissed their stale review April 12, 2023 18:21

Agree with @slaren — the space should be removed.

@prusnak
Copy link
Collaborator Author

prusnak commented Apr 12, 2023

Okay, so the space should be removed. What about newline at the end of prompt files? Should it be kept or removed too?

@slaren
Copy link
Collaborator

slaren commented Apr 12, 2023

It depends on the prompt but in these cases it should be removed, adding a new line at the end of a prompt should be a deliberate choice and not something forced by the editor.

@prusnak
Copy link
Collaborator Author

prusnak commented Apr 12, 2023

It depends on the prompt but in these cases it should be removed, adding a new line at the end of a prompt should be a deliberate choice and not something forced by the editor.

I am asking about the two particular prompt files I touched in this PR.

@slaren
Copy link
Collaborator

slaren commented Apr 12, 2023

I don't think these prompts should end in a new line.

@prusnak prusnak force-pushed the editorconfig-prompts branch from bc6a5ce to 9b52fc3 Compare April 13, 2023 08:52
@prusnak
Copy link
Collaborator Author

prusnak commented Apr 13, 2023

Thanks @slaren!

Updated the PR to reflect review comments. The files prompts/chat-with-bob.txt and prompts/reason-act.txt do not contain final newlines now. See 9b52fc3

@prusnak prusnak merged commit 82d146d into master Apr 13, 2023
@prusnak prusnak deleted the editorconfig-prompts branch April 13, 2023 09:33
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.

3 participants