zonyitoo / doubanfm-qt

A DoubanFM client
Other
511 stars 99 forks source link

Some doubts #36

Closed chenkrun closed 9 years ago

chenkrun commented 9 years ago

Hello, I am a newbie in Qt. I think your UI is beautiful and wonderful. When I run the program, I find some doubts:

  1. I think the QRegExp class which you use is old to use, it may lead to some unknown parsing questions, so I use QRegularExpression class instead.
  2. Some lyrics don not show at the lyric widget. When I debug, I find the timeReg may not include all time format of lyrics, eg. [00:00.000] or [00:00]. Thank you~
zonyitoo commented 9 years ago

Thanks for you suggestion.

I am very happy if you could submit a PR.

chenkrun commented 9 years ago

I am sorry, it's so late to reply your message.I don't know the 'PR' means...But I just modify two places in your codes:

  1. QRegEXP->QRegularExpression.
  2. timeReg->"[([0-9]{2}):([0-9]{2}).([0-9]{2,3})]|[([0-9]{2}):([0-9]{2})]", adding other possible situations.
zonyitoo commented 9 years ago

Pull Request (PR).

Or could you paste your modification here?

chenkrun commented 9 years ago

The modification mainly in your qlyricparser.cpp:

LyricList LyricParser::parse(QTextStream& stream) {

// [00:00.00] or [00:00.000] or [00:00]
QRegularExpression timeRegExp("\\[([0-9]{2}):([0-9]{2})\\.([0-9]{2,3})\\]|\\[([0-9]{2}):([0-9]{2})\\]");
QList<Lyric> result;
int zeroCount = 0;

while (!stream.atEnd())
{
    QString line = stream.readLine();
    QList<QTime> ticks;
    QRegularExpressionMatch match = timeRegExp.match(line);
    int length = 0;   
    while (match.hasMatch())
    {
        length = length + match.captured(0).size();
        QString minute = match.captured(1);
        QString second = match.captured(2);
        QString msecond = match.captured(3);

        if (msecond.isEmpty())
        {
            // This is a flag to determine if the time format is [00:00]. It will be used afterwords.
            hasmsecond = false;
            QTime time(0, match.captured(4).toInt(), match.captured(5).toInt(), 0);
            ticks.append(time);
        }
        else
        {
            QTime time(0, minute.toInt(), second.toInt(), msecond.toInt());
            ticks.append(time);
        }
        match = timeRegExp.match(line, length);
    }
    if (ticks.size() != 0)
        zeroCount++;
    QString lyricStr = line.right(line.size() - length);
    for (const QTime& t : ticks)
    {
        Lyric lyric;
        lyric.time = t;
        lyric.lyric = lyricStr;
        result.append(lyric);
    }
}
if (zeroCount == 0)
    hasmatched = false;// This is also a flag used afterwords.
std::sort(result.begin(), result.end(), [] (const Lyric& a, const Lyric& b) -> bool {
    return a.time < b.time;
});

return result;

}

zonyitoo commented 9 years ago

Please check this commit, is that what you mean?

chenkrun commented 9 years ago

Yeah, I am also a newbie in GitHub, so I don't know how to use it to write codes like what you do above. Here is a link to QRegExp vs QRegualrExpression, I think it maybe helpful. link: http://www.codeproject.com/Tips/729656/Reasons-to-abandon-and-replace-QRegExp-in-your-Qt Thank you for your reply.