[FFmpeg-devel] [PATCH v4 1/4] avutil/{avstring, bprint}: add XML escaping from ffprobe to avutil

[Jan Ekström]

On Mon, Feb 1, 2021 at 2:25 PM Nicolas George <george at nsup.org> wrote:
>
> Jan Ekström (12021-02-01):
> > I fear that those sound way, way too generic. "This escapes all XML"
> > definitely is not what that does.
> >
> > And yes, even if it has the better definition in the Doxygen comment.
>
> Why? Do you expect to add something that "escapes all XML"? What does it
> even mean?
>

No, I do not expect to. Just that as you made me add modifications to
this logic, I no longer wish to make it as simple as "XML escaping"
since at least in my opinion if nothing else - that would now include
those cases that have now been separated.

Of course I would *love* secondary opinions here. If other people
accept the naming then I will become less hesitant about them.

Also removing the attribute value part out of the attribute value
things seems like a dangerous bid. Same regarding others' opinions
with this as well.

> These symbols are what developers will need to type. Making them long is
> just a waste of time. Adding hard to remembers parts like "ATT" (is it
> "ATTR"? or "ATTRIBUTE"?) is just adding insult to injury.
>

I don't disagree with it being ugly, just that it is what it matches
in the spec. I wouldn't be against making it "ATTR_VALUE" or
"ATTRIBUTE_VALUE", but effectively does that improve anything?

Also for the record, I have clang-based auto-completion in most of my
editors, just for this. So that I can find things that I do not know
yet/recall.

> Make the documentation as clear as possible, but keep the symbols short
> and easy to remember.
>

As a general guide line I 100% agree with you. It's just that in this
case I'm not sure if removing details from the names like this is the
right way forward.

Jan