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,
On 1/21/21 10:10 AM, Pavel Sanda wrote:
On Thu, Jan 21, 2021 at 09:51:46AM -0500, Scott Kostyshak wrote:
On Thu, Jan 21, 2021 at 09:38:08AM +0200, Yuriy Skalko wrote:
Please review my recent patches for LyX.
Patch 1 (the Development.lyx patch) is good. Nice addition of the enum class.
Patch 4
On Thu, Jan 21, 2021 at 09:38:08AM +0200, Yuriy Skalko wrote:
> diff --git a/src/frontends/qt/DockView.h b/src/frontends/qt/DockView.h
> index 9c3a9e7460..1e7bd5f2db 100644
> --- a/src/frontends/qt/DockView.h
> +++ b/src/frontends/qt/DockView.h
> @@ -36,7 +36,7 @@ public:
>Qt::DockW
On Thu, Jan 21, 2021 at 09:51:46AM -0500, Scott Kostyshak wrote:
> On Thu, Jan 21, 2021 at 09:38:08AM +0200, Yuriy Skalko wrote:
> > Please review my recent patches for LyX.
>
> Patch 1 (the Development.lyx patch) is good. Nice addition of the enum class.
>
> Patch 4 also looks good. I thought it
Le 21/01/2021 à 08:38, Yuriy Skalko a écrit :
Please review my recent patches for LyX.
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
-
On Thu, Jan 21, 2021 at 09:38:08AM +0200, Yuriy Skalko wrote:
> Please review my recent patches for LyX.
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
Please review my recent patches for LyX.
Yuriy
From 9c9fd209149b05d6ec879d1792b9628734ce3ed5 Mon Sep 17 00:00:00 2001
From: Yuriy Skalko
Date: Sun, 10 Jan 2021 13:27:40 +0200
Subject: [PATCH 1/5] Update Development.lyx
---
lib/doc/Development.lyx | 99 +++--
Please review next 2 patches.
First one is fine.
In second one, can this:
@@ -1849,16 +1845,14 @@ docstring const
LaTeXFeatures::getTClassPreamble() const
-cit = usedInsetLayouts_.begin();
-end = usedInsetLayouts_.end();
+list::const_iterator cit = usedInsetLayouts_.begin();
+
On 12/6/20 6:01 AM, Yuriy Skalko wrote:
> Please review next 2 patches.
First one is fine.
In second one, can this:
@@ -1849,16 +1845,14 @@ docstring const
LaTeXFeatures::getTClassPreamble() const
- cit = usedInsetLayouts_.begin();
- end = usedInsetLayouts_.end();
+ list::const_iterator
Please review next 2 patches.
Yuriy
From 9d7d04d2fc0463bd1aa41acbbe22e95548a46d22 Mon Sep 17 00:00:00 2001
From: Yuriy Skalko
Date: Wed, 2 Dec 2020 22:34:28 +0200
Subject: [PATCH 1/5] More enums & includes refactoring
---
src/Changes.cpp | 1 +
src/Changes.h
Le 05/12/2020 à 18:05, Richard Kimberly Heck a écrit :
I just wanted to leave types of the pair as visible. Probably this is
not really important and the code should be shortened to your variant.
I tend to agree with Yuriy here. The code is a bit longer, but you do
get an explicit indication of
On 12/5/20 11:31 AM, Yuriy Skalko wrote:
>> I see they are in now, but I have a proposal (of style). I code like
>> below,
>> - for (; qq != end; ++qq) {
>> - docstring const style = from_ascii(qq->first);
>> - bool langdef = (style[0] == langqs);
>> -
I see they are in now, but I have a proposal (of style). I code like below,
- for (; qq != end; ++qq) {
- docstring const style = from_ascii(qq->first);
- bool langdef = (style[0] == langqs);
- bool globaldef = (style[0] == globalqsc);
+ map st
Le 02/12/2020 à 16:32, Yuriy Skalko a écrit :
Next refactoring patches.
I see they are in now, but I have a proposal (of style). I code like below,
- for (; qq != end; ++qq) {
- docstring const style = from_ascii(qq->first);
- bool langdef = (style[0] == langq
On Wed, Dec 02, 2020 at 12:01:15PM -0500, Richard Kimberly Heck wrote:
> On 12/2/20 10:32 AM, Yuriy Skalko wrote:
> > Next refactoring patches.
>
> #1 and #3 are fine. I'll leave #4 again to Pavel and others.
All patches look fine to me. Pavel
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
ht
#1 and #3 are fine. I'll leave #4 again to Pavel and others.
Thanks Riki. I'll wait to commit all at once.
In #2, we have:
/// who initiated the action
-Origin origin_;
+Origin origin_ = INTERNAL;
It doesn't look as if there was a default before.
It is set as defau
On 12/2/20 10:32 AM, Yuriy Skalko wrote:
Next refactoring patches.
#1 and #3 are fine. I'll leave #4 again to Pavel and others.
In #2, we have:
/// who initiated the action
- Origin origin_;
+ Origin origin_ = INTERNAL;
It doesn't look as if there was a default before.
I often w
Next refactoring patches.
From 884c8270bc31208d150b444140b116374e78a479 Mon Sep 17 00:00:00 2001
From: Yuriy Skalko
Date: Wed, 2 Dec 2020 00:16:55 +0200
Subject: [PATCH 1/4] Fix warnings and use range-based loop
---
src/frontends/qt/Menus.cpp | 14 ++
1 file changed, 6 insertions(+)
They look good.
JMarc
Thanks, committed.
Yuriy
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Le 30/11/2020 à 23:33, Yuriy Skalko a écrit :
And here are next 4 patches.
They look good.
JMarc
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
And here are next 4 patches.
Yuriy
From e89abcf0654343ec6090ace2bfe243809b64cabc Mon Sep 17 00:00:00 2001
From: Yuriy Skalko
Date: Mon, 30 Nov 2020 18:06:12 +0200
Subject: [PATCH 1/4] Remove useless breaks
---
src/insets/InsetNewpage.cpp | 8
1 file changed, 8 deletions(-)
diff --git
Looks good to me. Pavel
Thanks, committed.
Yuriy
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
On Mon, Nov 30, 2020 at 12:38:36PM +0200, Yuriy Skalko wrote:
> Next patches
Looks good to me. Pavel
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
Next patches
Yuriy
From 065bbc50691b50bcc6b608fbcfb362d7f0e654ab Mon Sep 17 00:00:00 2001
From: Yuriy Skalko
Date: Sat, 28 Nov 2020 01:13:36 +0200
Subject: [PATCH 1/4] Cleanup included headers
---
src/KeySequence.cpp | 2 --
src/frontends/qt/GuiErrorList.cpp | 1 -
src/frontends/
#1, #4, and #5 are all fine.
I think #3 is fine, too, but someone who knows more about that code than I
do should have a look, too.
I suspect #2 is fine, as well, but Pavel may want to weigh in.
2 looks fine, 3 looks fine in principle :)
Pavel
Thanks for reviewing, all committed.
Yuriy
--
On Fri, Nov 27, 2020 at 05:53:00PM -0500, Richard Kimberly Heck wrote:
> On 11/27/20 4:43 PM, Yuriy Skalko wrote:
> >>If it really isn't used, then go ahead.
> >>
> >>Riki
> >
> >Here is patch to remove it + another patches to review.
>
> #1,
On 11/27/20 4:43 PM, Yuriy Skalko wrote:
If it really isn't used, then go ahead.
Riki
Here is patch to remove it + another patches to review.
#1, #4, and #5 are all fine.
I think #3 is fine, too, but someone who knows more about that code than
I do should have a look, too.
I suspe
If it really isn't used, then go ahead.
Riki
Here is patch to remove it + another patches to review.
Yuriy
From a164646e24e8271950d51ccc6d889f860cddff5b Mon Sep 17 00:00:00 2001
From: Yuriy Skalko
Date: Fri, 27 Nov 2020 11:09:16 +0200
Subject: [PATCH 1/5] Use range-based loops
---
On 11/27/20 5:35 AM, Yuriy Skalko wrote:
>> All the patches are fine. I have only one remark concerning
>> Counters::copy. I do not understand why this is a Counters method
>> _and_ it receives two Counters instances as parameters. This looks
>> like a badly specified method.
>>
>>
>> OTOH, it look
All the patches are fine. I have only one remark concerning Counters::copy. I
do not understand why this is a Counters method _and_ it receives two Counters
instances as parameters. This looks like a badly specified method.
OTOH, it looks like it is not used. What about removing it ?
JMarc
Le 26/11/2020 à 22:07, Yuriy Skalko a écrit :
Here are next 5 patches.
All the patches are fine. I have only one remark concerning
Counters::copy. I do not understand why this is a Counters method _and_
it receives two Counters instances as parameters. This looks like a
badly specified metho
Here are next 5 patches.
Yuriy
From bdf2a935f79f465e75bafdedc3e50a4a513e24ca Mon Sep 17 00:00:00 2001
From: Yuriy Skalko
Date: Thu, 26 Nov 2020 00:17:29 +0200
Subject: [PATCH 1/5] Use to_string function
---
src/client/client.cpp | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
I do not like the removal of debug.h.
It's very handy to be able to simply lyxerr while debuging
without playing with includes...
Pavel
OK. I restored debug.h includes in headers and committed the patch.
Yuriy
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listi
This one comes from the same place. I'll adopt the same solution there.
Riki
Thanks for fixes.
Yuriy
--
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel
On Tue, Nov 24, 2020 at 10:55:41PM +0100, Pavel Sanda wrote:
> On Tue, Nov 24, 2020 at 04:49:59PM -0500, Richard Kimberly Heck wrote:
> > On 11/24/20 4:29 PM, Yuriy Skalko wrote:
> > >As for the patch 5, these null pointer dereferences happen even on opening
> > >LyX manuals. It is undefined behavi
On 11/24/20 4:55 PM, Pavel Sanda wrote:
On Tue, Nov 24, 2020 at 04:49:59PM -0500, Richard Kimberly Heck wrote:
On 11/24/20 4:29 PM, Yuriy Skalko wrote:
As for the patch 5, these null pointer dereferences happen even on opening
LyX manuals. It is undefined behavior even if it doesn't crash LyX.
On 11/24/20 4:59 PM, Richard Kimberly Heck wrote:
On 11/24/20 4:29 PM, Yuriy Skalko wrote:
As for the patch 5, these null pointer dereferences happen even on
opening LyX manuals. It is undefined behavior even if it doesn't
crash LyX. Most likely should be fixed somewhere instead of just
checki
On 11/24/20 4:29 PM, Yuriy Skalko wrote:
As for the patch 5, these null pointer dereferences happen even on
opening LyX manuals. It is undefined behavior even if it doesn't crash
LyX. Most likely should be fixed somewhere instead of just checking.
From 234bfe70c1e2766d856257aebe7eaad8836f5976 M
On Tue, Nov 24, 2020 at 04:49:59PM -0500, Richard Kimberly Heck wrote:
> On 11/24/20 4:29 PM, Yuriy Skalko wrote:
> >As for the patch 5, these null pointer dereferences happen even on opening
> >LyX manuals. It is undefined behavior even if it doesn't crash LyX. Most
> >likely should be fixed somew
On 11/24/20 4:29 PM, Yuriy Skalko wrote:
As for the patch 5, these null pointer dereferences happen even on
opening LyX manuals. It is undefined behavior even if it doesn't crash
LyX. Most likely should be fixed somewhere instead of just checking.
1-4 are fine. I'll have a look at the null poi
As for the patch 5, these null pointer dereferences happen even on
opening LyX manuals. It is undefined behavior even if it doesn't crash
LyX. Most likely should be fixed somewhere instead of just checking.
Yuriy
From 761602ef70c60403992255ac2e3c23b112235378 Mon Sep 17 00:00:00 2001
From: Yuriy
41 matches
Mail list logo
