Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-3985

QKeySequence::fromString bug

    XMLWordPrintable

Details

    Description

      I found a bug in QKeySequence::fromString - if there's an emacs-style shortcut
      without a space after the comma it produces a nonsense shortcut. the
      constructor has no such problem. test case attached.

      KShortcut uses fromString, which might explain some of my other odd
      experiences... I thought it was a kde bug at first

      I'm working on a patch. looks like really assign() should be used to avoid
      code duplication, but that means it'll have to take a format... odd that
      fromString and toString default to portable while decode uses native.

      I'm working off qt-copy from about a week ago - do tell me if this has been
      fixed already

      example:

      main.cpp
      #include <QKeySequence>
      #include <QDebug>
      
      void test(const QString& str)
      {
          qDebug() << str << "constructor" << QKeySequence(str).toString() << "fromString" << QKeySequence::fromString(str).toString();
      }
      
      int main(int argc, char **argv) {
          test("alt+d, l");
          test("alt+d,l");
      }
      

      patch:

      Index: qkeysequence.cpp
      ===================================================================
      --- qkeysequence.cpp (revision 952031)
      +++ qkeysequence.cpp (working copy)
      @@ -741,6 +741,8 @@
           Up to four key codes may be entered by separating them with
           commas, e.g. "Alt+X,Ctrl+S,Q".
       
      +    \a key should be in NativeText format.
      +
           This constructor is typically used with \link QObject::tr() tr
           \endlink(), so that shortcut keys can be replaced in
           translations:
      @@ -757,6 +759,16 @@
       }
       
       /*!
      +    \since 4.x
      +    Creates a key sequence from the \a key string based on \a format.
      +*/
      +QKeySequence::QKeySequence(const QString &key, QKeySequence::SequenceFormat format)
      +{
      +    d = new QKeySequencePrivate();
      +    assign(key, format);
      +}
      +
      +/*!
           Constructs a key sequence with up to 4 keys \a k1, \a k2,
           \a k3 and \a k4.
       
      @@ -902,9 +914,24 @@
           contain up to four key codes, provided they are separated by a
           comma; for example, "Alt+X,Ctrl+S,Z". The return value is the
           number of key codes added.
      +    \a keys should be in NativeText format.
       */
       int QKeySequence::assign(const QString &ks)
       {
      +    return assign(ks, NativeText);
      +}
      +
      +/*!
      +    \fn int QKeySequence::assign(const QString &keys, QKeySequence::SequenceFormat format)
      +    \since 4.x
      +
      +    Adds the given \a keys to the key sequence (based on \a format).
      +    \a keys may contain up to four key codes, provided they are
      +    separated by a comma; for example, "Alt+X,Ctrl+S,Z". The return
      +    value is the number of key codes added.
      +*/
      +int QKeySequence::assign(const QString &ks, QKeySequence::SequenceFormat format)
      +{
           QString keyseq = ks;
           QString part;
           int n = 0;
      @@ -933,7 +960,7 @@
               }
               part = keyseq.left(-1 == p ? keyseq.length() : p - diff);
               keyseq = keyseq.right(-1 == p ? 0 : keyseq.length() - (p + 1));
      -        d->key[n] = decodeString(part);
      +        d->key[n] = QKeySequencePrivate::decodeString(part, format);
               ++n;
           }
           return n;
      @@ -1384,12 +1411,7 @@
       */
       QKeySequence QKeySequence::fromString(const QString &str, SequenceFormat format)
       {
      -    QStringList sl = str.split(QLatin1String(", "));
      -    int keys[4] = {0, 0, 0, 0};
      -    int total = qMin(sl.count(), 4);
      -    for (int i = 0; i < total; ++i)
      -        keys[i] = QKeySequencePrivate::decodeString(sl[i], format);
      -    return QKeySequence(keys[0], keys[1], keys[2], keys[3]);
      +    return QKeySequence(str, format);
       }
       
       /*****************************************************************************
      Index: qkeysequence.h
      ===================================================================
      --- qkeysequence.h (revision 952031)
      +++ qkeysequence.h (working copy)
      @@ -139,8 +139,14 @@
               SaveAs
            };
       
      +    enum SequenceFormat {
      +        NativeText,
      +        PortableText
      +    };
      +
           QKeySequence();
           QKeySequence(const QString &key);
      +    QKeySequence(const QString &key, SequenceFormat format);
           QKeySequence(int k1, int k2 = 0, int k3 = 0, int k4 = 0);
           QKeySequence(const QKeySequence &ks);
           QKeySequence(StandardKey key);
      @@ -158,11 +164,6 @@
       #endif
           };
       
      -    enum SequenceFormat {
      -        NativeText,
      -        PortableText
      -    };
      -
           QString toString(SequenceFormat format = PortableText) const;
           static QKeySequence fromString(const QString &str, SequenceFormat format = PortableText);
       
      @@ -192,6 +193,7 @@
           static int decodeString(const QString &ks);
           static QString encodeString(int key);
           int assign(const QString &str);
      +    int assign(const QString &str, SequenceFormat format);
           void setKey(int key, int index);
       
           QKeySequencePrivate *d;
      

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            mitch_curtis Mitch Curtis
            janichol Andy Nichols
            Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Gerrit Reviews

                There are no open Gerrit changes