-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
XML mode for the cell editor #1275
Conversation
The new editor mode shares the same Scintilla widget as the JSON mode. The JsonTextEdit class has been generalised. Future modes supported by Scintilla could be added with the current pattern. As a consequence, the EditMode is not always equal to the current stacked widget. Some code in EditDialog has been refactored, so it is easier to understand and modified with so many modes. textNullSet has been replaced by the use of dataType as Null. SVG is promoted to a new recognised data type, so it can be edited in the XML mode. The XML data is formatted and validated following the pattern established by the JSON mode. New modules are needed by the XML mode: the Qt XML module and some new Scintilla files required by the HTML/XML lexer. The indent_compact was incorrectly named in Setting::getDefaultValue. See issue #1253.
That change was unwanted.
This sounds pretty awesome @mgrojo. 😄 |
Haha, that's a lot of code 😉 I'll try to review it soon-ish but it's going to take some time. I hope that's no problem for you. |
No problem at all. Take the time you deem necessary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you have a look at these comments? I couldn't find any major issues, so they are all minor points. The missing break statements cause double execution of some code and probably don't affect the data, but they are still worth fixing I guess 😄
Are you ok to resolve the merge conflicts too? 😉 I think then we are safe to merge this.
Sorry for taking so long but this huge amount of code was deterring me 😉 Turned out to be mostly Scintilla code but still... I wonder why they need a C++ lexer for the XML lexer...
emit recordTextUpdated(currentIndex, newData.toUtf8(), false); | ||
} | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing break;
after this line.
|
||
} | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And missing break;
after this line.
src/EditDialog.cpp
Outdated
dataLength = sciEdit->text().length(); | ||
break; | ||
} | ||
// ui->labelType->setText(tr("Type of data currently in cell: Text / Numeric")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we delete this line?
Set breaks in nested switch where missing. Fix editModeChanged since it was overwriting the dataType for JSON and SVG. Additionally, removed unnecessary conversion in fromJson.
# Conflicts: # src/EditDialog.cpp # src/ExtendedScintilla.cpp
Excellent. Only 2 PR's to go! I'd better get the SQLite math extension thing figured out soon then. 😄 |
@MKleusberg I'm still struggling with git. I had problems merging this and I think I overwrote your change to the qmake compilation, because I don't see it now in this pull request. I'm unable to compile with qmake, so I don't know what I must restore. Sorry. I definitely have to read one or more git tutorials again... |
@mgrojo I've just restored the deleted branch, so tomorrow (when I'm a bit more awake) I can look into this. If all of the source code bits are still around in places, I should be able to manually reconstruct things into a working order. 😄 And no worries with the git troubles. I friggin hated git when first using it, and it took me ages to grok. But got the hang of it eventually. You'll be fine. 😄 |
Also, some spaces have been replaced by tabs for uniformity. See issue #1309
@justinclift The problem is that overwrote this branch with my local branch. I didn't know why git didn't allow me to push the changes and I made some command for overwriting the remote branch, since I was convinced that the remote branch should be equal to mine. Later I remembered that Martin made some changes to the qmake compilation and then I realized the error 😳 Hopefully that was what I have just committed. |
This has a lot of changes and new dependencies, so I open a pull request for review and discussion.
The new editor mode shares the same Scintilla widget as the JSON mode.
The JsonTextEdit class has been generalised. Future modes supported by
Scintilla could be added with the current pattern. As a consequence, the
EditMode is not always equal to the current stacked widget.
Some code in EditDialog has been refactored, so it is easier to understand
and modified with so many modes. textNullSet has been replaced by the use
of dataType as Null.
SVG is promoted to a new recognised data type, so it can be edited in the
XML mode.
The XML data is formatted and validated following the pattern established
by the JSON mode.
New modules are needed by the XML mode: the Qt XML module and some new
Scintilla files required by the HTML/XML lexer.
The indent_compact was incorrectly named in Setting::getDefaultValue.
See issue #1253.