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

[Anton Khirnov]

Quoting Jan Ekström (2021-02-01 13:38:43)
> 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.

Why not have a single ESCAPE_MODE_XML mode and use flags to tweak what
specifically is or is not escaped?

> 
> > 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?

Mild pereference for "attr" here
Besides, doesn't everyone use auto-completion for identifiers?

-- 
Anton Khirnov