[PATCH] Patches to review

[Yuriy Skalko]

Thanks for all for reviewing the patches!


> Patch 1 (the Development.lyx patch) is good. Nice addition of the enum class.
> 
> Patch 4 also looks good. I thought it could break Qt 4.8 compilation but that's 
> not the case [1, 2].
> 
> Sorry that I don't know enough to look at the others.
> 
> Scott

Yes, it is Qt4.8-compatible. I have its docs installed and check 
compatibility before any usage of newer names.


> Concerning patch 1 and 5: what guideline do you use for using vector vs list. The patches look good, I am just wondering.
> 
> 
> Concerning patch 4: what does the replacement of 0 or nullptr by {} bring?
> 
> JMarc

You've meant patches 2 and 5? In patch 2, QList is a dynamic array 
similar to std::vector, so just use std library instead of Qt where it 
is not required. In patch 5, the container usage is just fits linked 
list the best -- insertions at the front and deleting in the middle are 
ineffective in vectors.

As for the patch 4 there are warnings about this from Qt.
Here I experimented with automatic LyX build using GitHub Actions: 
https://github.com/magistere/lyx/actions
Logs with warnings can be seen when you log in to GitHub.

Also there are several code scanning tools, that give some warnings. 
Maybe some will be useful to fix. They are available here (again only 
after logging in): https://github.com/magistere/lyx/security/code-scanning


> 2,3 is fine. 5 is fine unless we use direct [] access somewhere.
> 
> I know Riki did not announce it officially yet, but we should start looking at 
> fixing
> bugs which hinder us from 2.4 release rather than refactoring the codebase. I 
> know, boring...
> 
> Pavel

As I've checked, there is no random access for lastfiles container.


> Not part of your changes but the weird 3 line comment above could be just 
> single line
> 
> Pavel

Strict 80 chars per line setting in someone's editor. As I understand 
there is no such requirement, right? We can mention this in 
Development.lyx manual.


> I think I did send an email to that effect. In any event, my understanding is that Yuriy intends to commit these things to a feature branch for now. We don't want to risk another weird surprise like with the move constructor.
> 
> 
> Riki

Probably I violated this. I'm not sure if it is worth to create another 
branch for these patches. So let's just postpone them for some time. 
I'll commit the documentation patch only since it would be safe.


Yuriy