Skip to content

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

Merged
prusnak merged 1 commit into
masterfrom
editorconfig-prompts
Apr 13, 2023
Merged

prusnak merged 1 commit into
masterfrom
editorconfig-prompts

Conversation

@prusnak

@prusnak prusnak commented Apr 12, 2023

Copy link
Copy Markdown
Contributor

No description provided.

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

slaren commented Apr 12, 2023

Copy link
Copy Markdown
Member

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

prusnak commented Apr 12, 2023

Copy link
Copy Markdown
Contributor Author

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

@slaren

slaren commented Apr 12, 2023 •

Copy link
Copy Markdown
Member

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

prusnak commented Apr 12, 2023

Copy link
Copy Markdown
Contributor Author

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

slaren commented Apr 12, 2023

Copy link
Copy Markdown
Member

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

prusnak commented Apr 13, 2023 •

Copy link
Copy Markdown
Contributor Author

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
Seunghhon pushed a commit to Seunghhon/llama.cpp that referenced this pull request Apr 26, 2026
phuongncn pushed a commit to phuongncn/llama.cpp-gx10-dgx-sparks-deepseekv4 that referenced this pull request Apr 28, 2026
ljubomirj pushed a commit to ljubomirj/llama.cpp that referenced this pull request May 6, 2026
AlexiAlp pushed a commit to minghaop/llama.cpp that referenced this pull request Jun 2, 2026
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