D17144: Don't reverse order of files opened from the command line

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a reviewer: Kate.
ngraham added a comment.
This revision now requires changes to proceed.


  Thanks, this works great for both use cases! The code change itself looks sane, there are just a few style and formatting nitpicks I have. See the inline comments.

INLINE COMMENTS

> kateapp.cpp:178
>  
> -    Q_FOREACH(const QString positionalArgument, m_args.positionalArguments()) {
> +    // Fix to bug 397913: Don't reverse order of files opened from the command line
> +    QString positionalArgument;

This comment should describe why you're doing this, not just the bug. Something like `// Reverse the order here so the new tabs are opened in same order as the files were passed in on the command line`

> kateapp.cpp:457
>      Q_FOREACH(QJsonValue urlObject, urls) {
> +    
>          /**

Unnecessary/unrelated whitespace change

> main.cpp:419
>              // open given files...
> -            foreach(const QString & url, urls) {
> +            // BUG: 397913 Don't reverse order of files opened from the command line
> +            for (int i = urls.size()-1; i >= 0; i--) {

See above

REPOSITORY
  R40 Kate

REVISION DETAIL
  https://phabricator.kde.org/D17144

To: gmolteni, cfeck, zetazeta, ngraham, #kate
Cc: kwrite-devel, michaelh, ngraham, demsking, cullmann, sars, dhaumann
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kwrite-devel/attachments/20181124/1a4e3609/attachment.html>