📝 Add initial sphinx ext for per-doc feedbacks

[webknjaz]

Resolves #8783

[pradyunsg]

https://pip--8848.org.readthedocs.build/en/8848/

Uhhh... This is extremely invasive IMO.

[webknjaz]

@pradyunsg I thought so. But atm I'm just doing what I was asked to do. It's easier to reason about stuff when there's a preview :)

[webknjaz]

I've colored it a bit. I'll let @nlhkabu / @jdittrich / @georgiamoon decide how to deal with the invasiveness.

FWIW it's a good start and can be tweaked.

[jdittrich]

I'll let @nlhkabu / @jdittrich / @georgiamoon decide how to deal with the invasiveness.

Imho it is too invasive on such a short page, but totally OK on a longer page. We could hide one in case of short pages (don't know how hard this is). We could also make the box less big in general:

[McSinyx]

Suggested change

	1. What problem were you trying to solve when you came to this page?
	2. What content was useful?
	3. What content was not useful?
	1. What problem were you trying to solve when you came to this page?
	2. What content was useful?
	3. What content was not useful?

The rendered list seems to be over-indented, probably because triple-quotes preserve whitespace. I can't view the reST output (after this extension is applied) due to the theme (it's just not trivially possible), but I think generally it'd be safer to declare all these literals as module level constants to avoid accidentally messing up indentation.

[webknjaz]

I hear you but because it's also augmented with another line with context-specific var, I'm not sure if moving it on the module level is a good idea. After all, we need to keep some sort of common indentation level here and 3 lines below.

I guess I'll just experiment with dedenting both.

[pradyunsg]

I'd say: check if it's creating a blockquote in the final HTML. If it is, then you know that things are indented too far.

[McSinyx]

It is, and I was also concern about other texts provided as indented triple quoted literals, e.g. Did this article help? (relative to .. important). Sphinx is pretty relaxed and still renders the content even if it doesn't align with important, but I think it'd be easier to notice whitespace issues if the reST is provided as something close to unindented literals, which we can then str.format on.

[pfmoore]

I agree with @pradyunsg - this is too intrusive. Top or bottom is fine, but I'm not keen on both.

(Also, I think having the banner on the changelog page is probably a bit much, although I understand that it's an all-or-nothing thing because of the way it's implemented).

[pradyunsg]

It's not an all-or-nothing proposition since we have the document name in the code that's adding the $thing. We can have conditionals to "opt out" for certain pages, if we want.

[webknjaz]

It's not an all-or-nothing proposition since we have the document name in the code that's adding the $thing. We can have conditionals to "opt out" for certain pages, if we want.

Exactly, we can change this once there's a general consensus on what exactly is needed.

[webknjaz]

We could hide one in case of short pages (don't know how hard this is).

The easiest way of doing this is to maintain a list of exclusions I think.

We could also make the box less big in general

Feel free to send a CSS patch with a specific suggestion. "less big" is too broad and is not very well actionable...

[webknjaz]

@jdittrich oh, I just looked at the picture again. Are you suggesting to drop the list?

[jdittrich]

@jdittrich oh, I just looked at the picture again. Are you suggesting to drop the list?

Yes, that would be one way to make it smaller (which could be particularly important for the top box)

[webknjaz]

Alright, so just to clarify the action items now:

1. drop the list from the top box, but not the bottom one
2. dedent the list
3. implement page exclusions (not sure if everybody agrees on this one)

Anything I'm missing?

[webknjaz]

So I think I've made it a bit better:

1. Now the list is only present in the bottom box but not the top one (as requested by @jdittrich) -- see https://pip--8848.org.readthedocs.build/en/8848/development/
2. It is possible to exclude documents and I've added the news document to the exclude list (:wave: @pfmoore) -- see https://pip--8848.org.readthedocs.build/en/8848/news/
3. Adjusted the indentation (@McSinyx ✌️)
4. The top box is hidden if the document RST source has over 50 LOC (@jdittrich 🎉) -- see https://pip--8848.org.readthedocs.build/en/8848/
5. I've added a number of external toggles so that it's possible to adjust the following w/o the need to look into the monstrous extension implementation:
   • box (admonition) type
   • the "big doc" threshold
   • the target email
   • the excluded documents set
   • the questions list

[pradyunsg]

I like it. @nlhkabu and @jdittrich if you reckon this is good to go, let's ship it. :)

[nlhkabu]

Looks good to me! Thanks @webknjaz ! ❤️

[jdittrich]

thanks!
fine for me, too :)