Patch #1066 2006-05-22 17:01

pchris

Dynamic column width setting
Download
1066-Dynamic_column.patch (2.2 KB)
Category
Application::Refinement
Status
Rejected
Close date
2006-06-05 11:52
Assigned to
 
Index: src/sdk/cbeditor.cpp
===================================================================
--- src/sdk/cbeditor.cpp    (revision 2484)
+++ src/sdk/cbeditor.cpp    (working copy)
@@ -123,7 +123,8 @@
         m_ensure_consistent_line_ends(true),
         m_LastMarginMenuLine(-1),
         m_LastDebugLine(-1),
-        m_useByteOrderMark(false)
+        m_useByteOrderMark(false),
+        lineNumbersWidth(0)
     {
         m_encoding = wxLocale::GetSystemEncoding();
     }
@@ -248,6 +249,27 @@
         control->ConvertEOLs(control->GetEOLMode());
     }
 
+    /** Set line number column width */
+    void SetLineNumberColWidth()
+    {
+        int lineCount = 0;
+        lineCount = m_pOwner->m_pControl->GetLineCount();
+
+        int lineNumWidth = 1;
+        while (lineCount >= 10)
+        {
+            lineCount /= 10;
+            ++lineNumWidth;
+        }
+
+        if (lineNumWidth != lineNumbersWidth) {
+            int pixels = 0;
+            pixels = 5 + lineNumWidth * 8;
+            m_pOwner->m_pControl->SetMarginWidth(0, pixels);
+            lineNumbersWidth = lineNumWidth;
+        }
+    }
+
     //vars
     bool m_strip_trailing_spaces;
     bool m_ensure_final_line_end;
@@ -259,6 +281,8 @@
     wxFontEncoding m_encoding;
     bool m_useByteOrderMark;
 
+    int lineNumbersWidth;
+
 };
 ////////////////////////////////////////////////////////////////////////////////
 
@@ -350,8 +374,8 @@
 //    Manager::Get()->GetMessageManager()->DebugLog(_T("ctor: Filename=%s\nShort=%s"), m_Filename.c_str(), m_Shortname.c_str());
 
     CreateEditor();
+    m_IsOK = Open();
     SetEditorStyle();
-    m_IsOK = Open();
 
     // if !m_IsOK then it's a new file, so set the modified flag ON
     if (!m_IsOK && filename.IsEmpty())
@@ -579,7 +603,7 @@
     // line numbering
     m_pControl->SetMarginType(0, wxSCI_MARGIN_NUMBER);
        if (mgr->ReadBool(_T("/show_line_numbers"), true))
-        m_pControl->SetMarginWidth(0, 48);
+        m_pData->SetLineNumberColWidth();
     else
         m_pControl->SetMarginWidth(0, 0);
 
@@ -1870,6 +1894,9 @@
 
             line = m_pControl->MarkerPrevious(line - 1, 1 << BREAKPOINT_MARKER);
         }
+
+        // Adjust line numbers column
+        m_pData->SetLineNumberColWidth();
     }
 }
 
thomasdenk 2006-05-22 17:38

Not acceptable as it is, sorry.

First, the patch implements a behaviour that is the opposite of the standard behaviour and only wanted by a tiny fraction.

This would not be a problem as such if it were an option, but the patch completely supersedes the correct behaviour and does not offer any possibility to revert to the original.

Second, the width calculation is hardcoded using device-dependent coordinates (assuming a character width of 8 pixels). This will very likely not work well across different platforms.

Yes, the original implemetation hardcodes the column width too, but that is another situation. The width in the original implementation was chosen large enough to easily accomodate all "normal" cases.

pchris 2006-05-22 17:44

I understand what you mean.

The character width is my stupidness... It was intended to be dynamically allocated, however I forgot about it while testing. *sigh*

Is it an acceptable solution, if I add a checkbox to the Settings window saying "Set column width dynamically"?

thomasdenk 2006-05-22 18:30

With those two changes, I guess there is not much left to object.

Personally, I don't like the proposed feature at all (in fact I think it is misbehaviour), but then... to each according to his needs.

If it is configurable and if it is not the default, no problem. :)

Please update before making any changes, though, Sethjackson's last patch modified the XRC file, and it is already in the repository, so your modifications will provoke conflicts if you don't update first.

mandrav 2006-06-05 11:52

Applied a more recent patch (or so I think!).