[vlc-devel] [PATCH 2/3] Refactor RSS display string generation

Thomas Guillem thomas at gllm.fr
Wed Aug 19 20:06:36 CEST 2020



On Tue, Jul 21, 2020, at 01:18, Daniel Glaas wrote:
> I additionally notices a bug in the generation of a scrolling title. It 
> is fixed in the update attached patch.

Hello Daniel, your patch is doing a fix + a refactor. This makes it hard to review since your fix is hidden by your refactor.

Could you split your patch into 2 patches please? Refactor, then the fix.

Thanks in advance!
Thomas Guillem

> 
> Am 18.07.20 um 12:40 schrieb Daniel Glaas:
> > You mean that instead of snprintf() the function vlc_memstream() is used?
> >
> > Ok, it was not my purpose of this patch to replace the snprintf() 
> > calls with another functions. I just wanted to make it easier readable 
> > with function parameters are passed into snprintf(). You think that if 
> > these lines are touched, then the usage of vlc_memstream should be used?
> >
> > Am 18.07.20 um 09:38 schrieb Rémi Denis-Courmont:
> >>     Hi,
> >>
> >> Le lauantaina 18. heinäkuuta 2020, 5.05.14 EEST Daniel Glaas a écrit :
> >>> Refactor RSS display string generation
> >>>
> >>> The RSS display string generation contained lot of code 
> >>> duplications. With
> >>> commit, structures that are used several time are factor out to make 
> >>> the
> >>> final
> >>> snprintf() statements which write the text into the marquee buffer 
> >>> better
> >>> readable.
> >> Yes but we use vlc_memstream for that sort of things nowadays. It's far
> >> simpler and less error-prone.
> >>
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
> 
> *Attachments:*
>  * 0002-Refactor-RSS-display-string-generation.patch
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20200819/86faa31e/attachment.html>


More information about the vlc-devel mailing list