Max length on multiple line text control for GTK #23928
Conversation
There was a problem hiding this comment.
This and all the other submodules shouldn't be changed.
There was a problem hiding this comment.
@vadz ,
They shouldn't.
As explained in the OP, I thought I was pushing the to master.
| @@ -62,7 +62,7 @@ class WXDLLIMPEXP_CORE wxTextEntry : public wxTextEntryBase | |||
| void SendMaxLenEvent(); | |||
| bool GTKEntryOnInsertText(const char* text); | |||
| bool GTKIsUpperCase() const { return m_isUpperCase; } | |||
|
|
|||
| int GetMaxLength() const { return m_maxlen; } | |||
There was a problem hiding this comment.
If it's GTK-specific, it should have GTK prefix.
There was a problem hiding this comment.
@vadz,
I probably won't.
I plan to update Mac port with this..
| @@ -99,7 +99,7 @@ class WXDLLIMPEXP_CORE wxTextEntry : public wxTextEntryBase | |||
| // Call this from the overridden wxWindow::GTKIMFilterKeypress() to use | |||
| // GtkEntry IM context. | |||
| int GTKEntryIMFilterKeypress(GdkEventKey* event) const; | |||
|
|
|||
| int m_maxlen; | |||
There was a problem hiding this comment.
This should be defined below, with the other member variables and initialized in its declaration as all new fields should be in master.
| // the handler that does not connected after | ||
| gtk_text_buffer_delete( buffer, &offset, &end ); | ||
| // Without following line GTK3 produces warning | ||
| #if GTK_CHECK_VERSION( 3, 0, 0 ) |
There was a problem hiding this comment.
This is useless, all this code seems to be GTK 3-specific.
| wxCommandEvent event( wxEVT_TEXT_MAXLEN, win->GetId() ); | ||
| event.SetEventObject( win ); | ||
| event.SetString( win->GetValue() ); | ||
| win->HandleWindowEvent( event ); |
There was a problem hiding this comment.
It seems very suspicious that the result is not being used. Normally it is possible to increase the max length when this event happens.
| // reason, ("Run Last") it won't work in GTKOnInsertText() as it called from | ||
| // the handler that does not connected after | ||
| gtk_text_buffer_delete( buffer, &offset, &end ); | ||
| // Without following line GTK3 produces warning |
| // probably because the signal is documented as "Run Last". For exactly same | ||
| // reason, ("Run Last") it won't work in GTKOnInsertText() as it called from | ||
| // the handler that does not connected after | ||
| gtk_text_buffer_delete( buffer, &offset, &end ); |
There was a problem hiding this comment.
Ideally we should delete just enough to stay under maxlen and not all new entry.
| if( IsSingleLine() ) | ||
| return GTK_EDITABLE(m_text); | ||
| else | ||
| return nullptr; |
There was a problem hiding this comment.
Why is this necessary? We still shouldn't be calling this for multiline controls, it doesn't make sense as they don't have any GtkEditable.
It looks like this is a leftover and should be simply reverted.
| GtkEntry* const entry = (GtkEntry*)GetEditable(); | ||
| if (!GTK_IS_ENTRY(entry)) | ||
| return; | ||
|
|
||
| gtk_entry_set_max_length(entry, len); | ||
| m_maxlen = len; | ||
| if (entry) | ||
| gtk_entry_set_max_length(entry, len); |
vadz
added
work needed
|
@vadz , Thank you. |
The errors happened much earlier. Search the logs for "error:". |
Thx. I presume they are here because I pushed the Scintilla update. Is it possible to fix them somehow? I'm not that proficient with Git... |
It sure looks like they are related to Scintilla. Yes, it is possible to fix them, but it might be easier for you to create a fresh clone of master and manually re-apply your changes there. |
|
@swt2c ,
Well re-clone will probably be too harsh. ;-) This will probably work the same. |
The text control on Windows supports this, so this PR will bring it in par.
This PR resolves issue 17772.
Only 2 last commits are relevant. Sorry - I thought I was updating master.
Vadim didn't have time back then to finish it. Hopefully he will now. Or Paul.
Hopefully either he or Paul will be able to comment.
What doesn't work here is pasting in the multi-line text control.
I look at implementation, but I'm not sure how to best implement that.
Please review.
I can try to take a look at pasting, but it will take some time..
Thank you.