--===============1375059443829623873== Content-Type: multipart/alternative; boundary="===============5796403929052478401==" --===============5796403929052478401== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115926/#review50412 ----------------------------------------------------------- Looks almost good. Let's see what the holy belliness says to this. part/utils/katecolors.h we could start with 0, then you wouldn't need the "- 1" offset in KateSchemaConfigColorTab::colorItemList. My suggestion: enum Mark { Bookmark = 0, ActiveBreakpoint, ReachedBreakpoint, DisabledBreakpoint, Execution, Warning, Error, MarkCount FIRST_MARK = Bookmark, }; Then you can write as always: for (i = 0, i < MarkCount; ++i) part/utils/kateconfig.cpp same here, I don't like =1 and the -1 in mark[] :-) - Dominik Haumann On Feb. 20, 2014, 10:19 p.m., Milian Wolff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/115926/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2014, 10:19 p.m.) > > > Review request for Kate and Christoph Cullmann. > > > Repository: kate > > > Description > ------- > > Instead of copy'n'pasting the generation of default colors, we put that into a > central place and generate the colors there. > > Building on top of this patch, I plan to add some auto-tinting code as I use in > KDevelop for the currently hardcoded colors (see Qt::yellow & friends). > > Please accept this for KDE4, as it's a bug fix for people like me. > > > Diffs > ----- > > part/CMakeLists.txt 737f325 > part/schema/kateschemaconfig.cpp 541be03 > part/utils/kateconfig.cpp 65e8e8e > part/utils/katedefaultcolors.h PRE-CREATION > part/utils/katedefaultcolors.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/115926/diff/ > > > Testing > ------- > > Still looks as before to my eyes. Dunno if I regressed anything? > > > Thanks, > > Milian Wolff > > --===============5796403929052478401== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115926/

Looks almost good. Let's see what the holy belliness says to this.

part/utils/katecolors.h (Diff revision 1)
59
    Bookmark = 1,
we could start with 0, then you wouldn't need the "- 1" offset in KateSchemaConfigColorTab::colorItemList.

My suggestion:
enum Mark {
    Bookmark = 0,
    ActiveBreakpoint,
    ReachedBreakpoint,
    DisabledBreakpoint,
    Execution,
    Warning,
    Error,
    MarkCount

    FIRST_MARK = Bookmark,
  };

Then you can write as always:
for (i = 0, i < MarkCount; ++i)

part/utils/kateconfig.cpp (Diff revision 1)
void KateRendererConfig::setSchemaInternal( const QString &schema )
2232
  for (int i = 1; i <= KTextEditor::MarkInterface::reservedMarkersCount(); i++) {
2206
  for (int i = KateColors::FIRST_MARK; i <= KateColors::LAST_MARK; i++) {
same here, I don't like =1 and the -1 in mark[] :-)

- Dominik Haumann


On February 20th, 2014, 10:19 p.m. UTC, Milian Wolff wrote:

Review request for Kate and Christoph Cullmann.
By Milian Wolff.

Updated Feb. 20, 2014, 10:19 p.m.

Repository: kate

Description

Instead of copy'n'pasting the generation of default colors, we put that into a
central place and generate the colors there.

Building on top of this patch, I plan to add some auto-tinting code as I use in
KDevelop for the currently hardcoded colors (see Qt::yellow & friends).

Please accept this for KDE4, as it's a bug fix for people like me.

Testing

Still looks as before to my eyes. Dunno if I regressed anything?

Diffs

  • part/CMakeLists.txt (737f325)
  • part/schema/kateschemaconfig.cpp (541be03)
  • part/utils/kateconfig.cpp (65e8e8e)
  • part/utils/katedefaultcolors.h (PRE-CREATION)
  • part/utils/katedefaultcolors.cpp (PRE-CREATION)

View Diff

--===============5796403929052478401==-- --===============1375059443829623873== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ KWrite-Devel mailing list KWrite-Devel@kde.org https://mail.kde.org/mailman/listinfo/kwrite-devel --===============1375059443829623873==--