-
Notifications
You must be signed in to change notification settings - Fork 120
Add more representations of unprintable bytes #380
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,6 +24,7 @@ | |
| #include <QMouseEvent> | ||
| #include <QStaticText> | ||
| #include <QStringList> | ||
| #include <QTextCodec> | ||
|
|
||
| #include "ui/createchunkdialog.h" | ||
| #include "ui/fileblobmodel.h" | ||
|
|
@@ -71,6 +72,7 @@ class HexEdit : public QAbstractScrollArea { | |
| void applyChanges(); | ||
| void undo(); | ||
| void discardChanges(); | ||
| void setUnprintablesMode(QAction* action); | ||
|
|
||
| protected: | ||
| void paintEvent(QPaintEvent* event) override; | ||
|
|
@@ -177,6 +179,8 @@ class HexEdit : public QAbstractScrollArea { | |
| QScopedPointer<util::encoders::TextEncoder> textEncoder_; | ||
| util::EditEngine edit_engine_; | ||
|
|
||
| QString unprintablesModeString; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a private field, so it should be named
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
|
||
| void recalculateValues(); | ||
| void initParseMenu(); | ||
| void adjustBytesPerRowToWindowSize(); | ||
|
|
@@ -188,7 +192,10 @@ class HexEdit : public QAbstractScrollArea { | |
| WindowArea pointToWindowArea(QPoint pos); | ||
| QString addressAsText(qint64 pos); | ||
| QString hexRepresentationFromByte(uint64_t byte_val); | ||
| static QString asciiRepresentationFromByte(uint64_t byte_val); | ||
|
|
||
| QString asciiRepresentationFromByte(uint64_t byte_val); | ||
| void updateAsciiCache(); | ||
| void updateHexCache(); | ||
|
|
||
| static QColor byteTextColorFromByteValue(uint64_t byte_val); | ||
| QColor byteBackroundColorFromPos(qint64 pos, bool modified); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ | |
| */ | ||
| #pragma once | ||
|
|
||
| #include <QString> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Separate this includes with a blank line (see https://google.github.io/styleguide/cppguide.html#Names_and_Order_of_Includes)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
|
|
||
| namespace veles { | ||
| namespace util { | ||
| namespace settings { | ||
|
|
@@ -25,6 +27,8 @@ int columnsNumber(); | |
| void setColumnsNumber(int number); | ||
| bool resizeColumnsToWindowWidth(); | ||
| void setResizeColumnsToWindowWidth(bool on); | ||
| QString unprintablesModes(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMO it should be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, typo. Thanks for catching. |
||
| void setUnprintablesMode(QString mode); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rewritten. |
||
|
|
||
| } // namespace hexedit | ||
| } // namespace settings | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,8 +29,11 @@ | |
| #include "util/encoders/factory.h" | ||
| #include "util/misc.h" | ||
| #include "util/random.h" | ||
| #include "util/settings/hexedit.h" | ||
| #include "util/settings/theme.h" | ||
|
|
||
| #include <unistd.h> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can't use this header, it's non-standard (and as a result it doesn't compile on Windows, for example).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a debugging leftover, removed. |
||
|
|
||
| using veles::util::misc::array_size; | ||
|
|
||
| namespace veles { | ||
|
|
@@ -142,16 +145,9 @@ HexEdit::HexEdit(FileBlobModel* dataModel, QItemSelectionModel* selectionModel, | |
| recalculateValues(); | ||
|
|
||
| // Initialize hex & ASCII text cache. | ||
| for (size_t i = 0; i < array_size(hex_text_cache_); i++) { | ||
| hex_text_cache_[i].setPerformanceHint(QStaticText::ModerateCaching); | ||
| hex_text_cache_[i].setText(hexRepresentationFromByte(i)); | ||
| hex_text_cache_[i].setTextFormat(Qt::PlainText); | ||
| } | ||
| for (size_t i = 0; i < array_size(ascii_text_cache_); i++) { | ||
| ascii_text_cache_[i].setPerformanceHint(QStaticText::ModerateCaching); | ||
| ascii_text_cache_[i].setText(asciiRepresentationFromByte(i)); | ||
| ascii_text_cache_[i].setTextFormat(Qt::PlainText); | ||
| } | ||
| unprintablesModeString = util::settings::hexedit::unprintablesModes(); | ||
| updateAsciiCache(); | ||
| updateHexCache(); | ||
|
|
||
| connect(verticalScrollBar(), &QAbstractSlider::valueChanged, this, | ||
| &HexEdit::recalculateValues); | ||
|
|
@@ -475,6 +471,21 @@ HexEdit::HexEdit(FileBlobModel* dataModel, QItemSelectionModel* selectionModel, | |
| }); | ||
| } | ||
|
|
||
| void HexEdit::updateAsciiCache() { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please place this method in the same place as it was in the header file.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| for (size_t i = 0; i < array_size(ascii_text_cache_); i++) { | ||
| ascii_text_cache_[i].setPerformanceHint(QStaticText::ModerateCaching); | ||
| ascii_text_cache_[i].setText(asciiRepresentationFromByte(i)); | ||
| ascii_text_cache_[i].setTextFormat(Qt::PlainText); | ||
| } | ||
| } | ||
|
|
||
| void HexEdit::updateHexCache() { | ||
| for (size_t i = 0; i < array_size(hex_text_cache_); i++) { | ||
| hex_text_cache_[i].setPerformanceHint(QStaticText::ModerateCaching); | ||
| hex_text_cache_[i].setText(hexRepresentationFromByte(i)); | ||
| hex_text_cache_[i].setTextFormat(Qt::PlainText); | ||
| } | ||
| } | ||
| QModelIndex HexEdit::selectedChunk() { | ||
| if (chunkSelectionModel_ == nullptr) { | ||
| return {}; | ||
|
|
@@ -609,6 +620,12 @@ void HexEdit::discardChanges() { | |
| viewport()->update(); | ||
| } | ||
|
|
||
| void HexEdit::setUnprintablesMode(QAction* action) { | ||
| unprintablesModeString = action->text(); | ||
| updateAsciiCache(); | ||
| viewport()->update(); | ||
| } | ||
|
|
||
| qint64 HexEdit::selectionStart() { | ||
| if (selection_size_ < 0) { | ||
| return current_position_ + selection_size_ + 1; | ||
|
|
@@ -643,10 +660,32 @@ QString HexEdit::addressAsText(qint64 pos) { | |
| } | ||
|
|
||
| QString HexEdit::asciiRepresentationFromByte(uint64_t byte_val) { | ||
| if (byte_val >= 0x20 && byte_val < 0x7f) { | ||
| return QChar::fromLatin1(byte_val); | ||
| static QTextCodec* windows1250 = QTextCodec::codecForName("windows-1250"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should check whether
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moved the codec to class. Checking for nullity added. Todo added in ctor to display warning when codec in not available.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks ok now. |
||
|
|
||
| if (unprintablesModeString.compare("windows-1250") == 0) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This string is taken right from the UI string. Please don't do this - if someone will try to change the text in the UI the code will silently stop to work. Just use an enum.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
| // 0x85 is NEL, 0xa0 is nbsp in unicode, but we want to display them | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please add a dot after the sentence, now this line and the next looks like a one log sentence ;)
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced comments with better code. |
||
| // 0x83, ..., 0x98 are undefined in used codeing | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Strange mistake. Fixed. |
||
| if (byte_val != 0x85 && byte_val != 0xa0 && | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please also handle
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0x81 is also undefined. Please look at the table here: https://www.charset.org/charsets/windows-1250
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Now returns dot on the beginning. |
||
| (QChar(static_cast<uint>(byte_val)).isSpace() || | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cast it to unsigned char or and with 0xFF (so that
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bytes which are either undefined or translates to whitespace in CP1250 are displayed as space. |
||
| QChar(static_cast<uint>(byte_val)).isNull() || byte_val == 0x83 || | ||
| byte_val == 0x88 || byte_val == 0x90 || byte_val == 0x98)) { | ||
| return QChar(static_cast<uint>(' ')); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cannot cast char to char*. But " " works. |
||
| } | ||
|
|
||
| // printable ascii chars | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| if (byte_val >= 0x20 && byte_val < 0x7f) { | ||
| return QChar::fromLatin1(byte_val); | ||
| } | ||
|
|
||
| // ascii-unprintable chars | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| char a = (static_cast<char>(byte_val)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This conversion is implementation-defined for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| return byte_val >= 0x7f | ||
| ? windows1250->toUnicode(&a, 1) | ||
| : QChar(static_cast<uint>(byte_val) + 0x180); // greek | ||
| } | ||
| return "."; | ||
| // dots fallback | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's rather a "dots mode" ;)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
| return (byte_val >= 0x20 && byte_val < 0x7f) ? QChar::fromLatin1(byte_val) | ||
| : QString("."); | ||
| } | ||
|
|
||
| QColor HexEdit::byteTextColorFromByteValue(uint64_t byte_val) { | ||
|
|
@@ -814,7 +853,7 @@ void HexEdit::paintEvent(QPaintEvent* event) { | |
| auto bgc = byteBackroundColorFromPos( | ||
| byte_idx, modified_positions[byte_idx - start_byte]); | ||
| bool redraw_hex = invalidated_rect.intersects(hex_rect); | ||
| bool redraw_ascii = invalidated_rect.intersects(ascii_rect); | ||
| bool redraw_ascii = true; // invalidated_rect.intersects(ascii_rect); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A leftover from debugging? ;)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, removed. |
||
| if (redraw_hex && bgc.isValid()) { | ||
| painter.fillRect(hex_rect, bgc); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -84,6 +84,11 @@ HexEditWidget::HexEditWidget( | |
| setParserIds(dynamic_cast<VelesMainWindow*>( | ||
| MainWindowWithDetachableDockWidgets::getFirstMainWindow()) | ||
| ->parsersList()); | ||
|
|
||
| initUnprintablesMenu(); | ||
| connect(&unprintables_menu_, &QMenu::triggered, hex_edit_, | ||
| &HexEdit::setUnprintablesMode); | ||
|
|
||
| selectionChanged(0, 0); | ||
| } | ||
|
|
||
|
|
@@ -282,6 +287,18 @@ void HexEditWidget::createToolBars() { | |
| addToolBar(edit_tool_bar_); | ||
|
|
||
| view_tool_bar_ = new QToolBar(tr("View")); | ||
|
|
||
| auto unprintables_tool_button = new QToolButton(); | ||
| unprintables_tool_button->setMenu(&unprintables_menu_); | ||
| unprintables_tool_button->setPopupMode(QToolButton::InstantPopup); | ||
| unprintables_tool_button->setIcon(QIcon(":/images/brightness.png")); | ||
| unprintables_tool_button->setText(tr("&Unprintables")); | ||
| unprintables_tool_button->setToolTip(tr("Appearance of unprintable bytes")); | ||
| unprintables_tool_button->setAutoRaise(true); | ||
| auto unprintables_widget_action = new QWidgetAction(view_tool_bar_); | ||
| unprintables_widget_action->setDefaultWidget(unprintables_tool_button); | ||
|
|
||
| view_tool_bar_->addAction(unprintables_widget_action); | ||
| view_tool_bar_->addAction(remove_column_act_); | ||
| view_tool_bar_->addAction(add_column_act_); | ||
| view_tool_bar_->addSeparator(); | ||
|
|
@@ -302,6 +319,13 @@ void HexEditWidget::initParsersMenu() { | |
| } | ||
| } | ||
|
|
||
| void HexEditWidget::initUnprintablesMenu() { | ||
| unprintables_menu_.clear(); | ||
| unprintables_menu_.addAction("dots"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rewritten. |
||
| unprintables_menu_.addSeparator(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why a separator here? We have only two options, so it not too useful now ;)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed. |
||
| unprintables_menu_.addAction("windows-1250"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rewritten. |
||
| } | ||
|
|
||
| void HexEditWidget::createSelectionInfo() { | ||
| auto* widget_action = new QWidgetAction(this); | ||
| auto* selection_panel = new QWidget; | ||
|
|
||
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.
Why it receives
QAction*when it in fact usesQStringfrom the inside? Signature of this function looks very unintuitive now, it should rather take the string argument.Update:
You should use enum instead of a string.
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.
This is slot for QMenu signal triggered of type void(QAction*) . AFAIK these must have the same type.
Enum instead of string: I'd have to bind somehow action in menu with enum value. Is there any well seen method?
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.
You can connect to a lambda which translates the arguments to ones that make more sense. Regarding enum: you can bind each of these actions to a lambda which calls setUnprintablesMode with a proper argument.
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.
Done. Not sure if capturing this by value is ok.
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.
It's ok, because it's a pointer to an object which will live longer than the lambda.