Improve: use constants to index into special exit columns in dlgRoomExits#5505
Merged
SlySven merged 5 commits intoMudlet:developmentfrom Oct 13, 2021
Conversation
…olumns This will make changing/adding the columns easier as part of what was created as the draft (and to be replaced) PR Mudlet#5308 - that is to be broken up into smaller, easier to digest, chunks! 8-) Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
…ditors I can't see how to share a single set of index definitions between the `dlgRoomExit` class and the `exitstreewidget` one - the latter is a base class on which the former depends but getting the one to inherit from the other is beyond me... As it happens I spotted the need for the second set when I reviewed `(void) ExitsTreeWidget::keyPressEvent(QKeyEvent*)` and spotted that I had not revised it to close the correct persistent editors in the merge commit 83db9f9 (part of PR Mudlet#439) seven years ago! Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
|
Hey there! Thanks for helping Mudlet improve. 🌟 Test versionsYou can directly test the changes here:
No need to install anything - just unzip and run. |
Contributor
|
clang-tidy review says "All clean, LGTM! 👍" |
6 tasks
Member
Would the "friend" mechanic in C++ help here? |
vadi2
reviewed
Oct 11, 2021
Member
vadi2
left a comment
There was a problem hiding this comment.
Definitely a welcome improvement 👍
Member
Author
😲 Of course - that is what I needed... |
This is so that all the (`static const`) `colIndex_Xxxx` values defined in the latter can also be used in the former (so as to remove the previous duplication I had to use)! Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
Contributor
Member
|
need to merge development branch back into this PR so it can run the new checks. |
…lExitColumnsIn_dlgRoomExits Needed to pick up the danger.js stuff recently inserted into the CI builds. Signed-off by: Stephen Lyons <slysven@virginmedia.com>
Signed-off-by: Stephen Lyons <slysven@virginmedia.com>
vadi2
approved these changes
Oct 12, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This will make changing/adding the columns easier as part of what was created as the draft (and to be replaced) PR #5308 - that is to be broken up into smaller, easier to digest, chunks! 8-)
I also needed to duplicate the column indexes in a second class but I can't see how to the share a single set of index definitions between theThere is a single set of constants which are defined in thedlgRoomExitclass and theexitstreewidgetone - the latter is a base class on which the former depends but getting the one to inherit from the other is beyond me...exitstreewidgetbut used also by thedlgRoomExitone by virtue of making it a friend of the former.As it happens I spotted the need for
the second setboth classes to have these constants when I reviewed(void) ExitsTreeWidget::keyPressEvent(QKeyEvent*)and spotted that I had not revised it to close the correct persistent editors in the merge commit 83db9f9 (part of PR #439) seven years ago!Signed-off-by: Stephen Lyons slysven@virginmedia.com
Release post highlight
None - I think - this is an internal tweak to make a subsequent PR easier to follow
Edited
Following ff6deef ensure the text squashed into the final commit message for this PR reflects the revised text!