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

Check for '%s' in prompt template #446

Closed
wants to merge 1 commit into from

Conversation

fcollonval
Copy link
Contributor

This fixes #442.

@mgeier yes this was what I meant. Here is a proposal.

@mgeier
Copy link
Member

mgeier commented Apr 29, 2020

Thanks, I think this would work. There can be a few theoretical edge cases where this wouldn't work, but I think nothing that's problematic in practice.

But ... is this feature really necessary?

You were suggesting this in #442 because you wanted to use it to remove the prompts.
However, this is not a good way to remove the prompts anyway. If you want to remove the prompts, you should use something like that: https://nbsphinx.readthedocs.io/en/latest/custom-css.html

Is there any other situation where this PR would be needed?

@fcollonval
Copy link
Contributor Author

Good question indeed. The only case I see is if the user wants to print something generic like "In" and "Out" next to each cells. But is it really useful... no sure. So I let you judge on this one and if you choose to simply close this, not problem for me (less trick, less bugs 😉 - especially if you already have some edge cases in mind.)

@mgeier
Copy link
Member

mgeier commented Apr 30, 2020

The only case I see is if the user wants to print something generic like "In" and "Out" next to each cells. But is it really useful... no sure.

I don't think so.

If you don't need it, there is no reason to implement this right now.

If anybody needs this at some point (which I don't think will happen), we can merge it then.

less trick, less bugs

Very true!

especially if you already have some edge cases in mind.

Those are only very vague edge cases ... a user might use different placeholders instead of %s. For example %r. Or %x if they want hexadecimal numbers!

@mgeier
Copy link
Member

mgeier commented May 7, 2020

I'm closing this for now, but if a real use case comes up, please let me know!

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.

Error if %s is not present in prompt string format
2 participants