Thread: Browsing through history in Editbox

  1. #1
    Registered User
    Join Date
    Dec 2004
    Location
    The Netherlands
    Posts
    91

    Browsing through history in Editbox

    Hi all,

    I am trying to implent this into an editbox, I am totally lost after editing the source several times, any feedback or info is much appreciated. I hope someone can point me to the right direction..

    I have got this so far, I uploaded my whole MS VC++ 6.0 project, if that is no good I can post snippets instead..

    Source Link

  2. #2
    Registered User Queatrix's Avatar
    Join Date
    Apr 2005
    Posts
    1,342
    How about zipping? I can't open rar files. (And I'm sure some others can't either.)

  3. #3
    Registered User
    Join Date
    Dec 2004
    Location
    The Netherlands
    Posts
    91

  4. #4
    Registered User Queatrix's Avatar
    Join Date
    Apr 2005
    Posts
    1,342
    Some things I noticed:

    You clear the edit before getting it:
    Code:
    char *buf;
    int buflen = GetWindowTextLength(hWnd);
    if(buflen == 0) return FALSE;
    SetDlgItemText(GetParent(hWnd), IDC_COMMAND, "");
    buf = malloc(sizeof(char)*++buflen);
    GetDlgItemText(GetParent(hWnd), IDC_COMMAND, buf, buflen);
    // Should be here
    And, you have:
    Code:
    Commands[ComIndex] = buf;
    /* ComLast = (ComIndex == 0) ? (15) : (ComIndex - 1); */
    free(buf);
    return TRUE;
    But char* type is a pointer, so freeing buf after assignment is making Commands[] pointer invalid. So, either, you don't want to free buf, Or you should assign Commands[] with strcpy(). Regardless, you need to free the whole Commands[] array when you close the program.

    Those are just some things that I noticed, so there could me more.
    Last edited by Queatrix; 05-23-2007 at 06:26 AM.

  5. #5
    Woof, woof! zacs7's Avatar
    Join Date
    Mar 2007
    Location
    Australia
    Posts
    3,459
    Your also assuming that memory allocation is going to work 100% of the time, (malloc returns NULL on failure).
    IE
    Code:
    if((buf = malloc(size)) == NULL)
    {
        /* malloc failed, do whatever */
    }
    However, I would simply recommend limiting the length of the edit box and then store 'buf' on the stack.
    Code:
    #define MAXBUF 255
    /* ... */
    char buf[MAXBUF];
    Also I'm pretty sure GetWindowTextLength doesn't return the how big the value of an edit box is in bytes but rather the 'visable' bytes. (See MSDN).

  6. #6
    Registered User
    Join Date
    Dec 2004
    Location
    The Netherlands
    Posts
    91
    Alright, I changed to what you guys said, I think only the looping trough commands (via VK_UP and VK_DOWN) should be fixed... The ComIndex should be increased when VK_UP is pressed and decreased when VK_DOWN is pressed, am I right?

  7. #7
    Yes, my avatar is stolen anonytmouse's Avatar
    Join Date
    Dec 2002
    Posts
    2,544
    I edited your code with notes added (untested).
    Code:
    char *Commands[16] = { NULL };
    /* ComIndex is our storage index, ComLast is our retrieval index. */
    int ComIndex = 0, ComLast = 0;
    
    LRESULT CALLBACK EditBox(HWND hWnd, UINT Message, WPARAM wParam, LPARAM lParam)
    {
    	if(Message == WM_KEYDOWN)
    	{
    		if(wParam == VK_RETURN) {
    			
    			char *buf;
    
    			int buflen = GetWindowTextLength(hWnd);
    
    			if (buflen == 0) return FALSE;
    
    			buf = malloc(sizeof(char) * ++buflen);
    			/* Note: Should check result of malloc here. */
    
    			/* Note: We have to get the text before we delete it.
    			 *       Could use GetWindowText instead. */
    			GetDlgItemText(GetParent(hWnd), IDC_COMMAND, buf, buflen);
    
    			/* Note: Could be easier to use SetWindowText. */
    			SetDlgItemText(GetParent(hWnd), IDC_COMMAND, "");
    
    			/* Free the old memory pointer if it exists. */
    			if (Commands[ComIndex]) free(Command[ComIndex]);
    
    			/* Store the buf memory pointer in the Command array. */
    			Commands[ComIndex] = buf;
    
    			/* Set retrieval slot to last stored item. */
    			ComLast = ComIndex;
    
    			/* Move storage slot to next slot in array. */
    			ComIndex++;
    
    			/* Loop to start if at end of array. */
    			if (ComIndex == 16) ComIndex = 0;
    
    			return TRUE;
    		}
    		else if(wParam == VK_UP) {
    			
    			if(!Commands[ComLast]) {
    				/* No item available! */
    				return FALSE;
    			}
    			else {
    				SetDlgItemText(GetParent(hWnd), IDC_COMMAND, Commands[ComLast]);
    			}
    
    			/* Move the retrieval index up one. */
    			ComLast = ComLast - 1;
    
    			/* Loop to end if required. */
    			if (ComLast == -1) ComLast = 15;
    		}

  8. #8
    Registered User
    Join Date
    Dec 2004
    Location
    The Netherlands
    Posts
    91
    I am having trouble with the down arrow button, shouldnt it look like this:
    Code:
    		else if(wParam == VK_DOWN) {
    			if(!Commands[ComLast]) {
    				/* No item available! */
    				return FALSE;
    			}
    			else {
    				SetDlgItemText(GetParent(hWnd), IDC_COMMAND, Commands[++ComLast]);
    			}
    
    			if (ComLast == 16) ComLast = 0;
    		}

  9. #9
    Malum in se abachler's Avatar
    Join Date
    Apr 2007
    Posts
    3,195
    Quote Originally Posted by Queatrix View Post
    How about zipping? I can't open rar files. (And I'm sure some others can't either.)
    why should we go through the extra effort to please you if you are too lazy to download WinRar. Its way better than 'ZIP', and its been the standard now for over 10 years.

  10. #10
    Registered User Queatrix's Avatar
    Join Date
    Apr 2005
    Posts
    1,342
    Chill out. At least I helped, all you did was clutter this thread with a useless/helpless complaint.

  11. #11
    Registered User
    Join Date
    Dec 2004
    Location
    The Netherlands
    Posts
    91
    I dont really mind putting them into a zip and uploading, anyways to get back on topic this is what I got so far, it works okay but there are a few problems.

    It browses okay trough the buffer, however when you press the up arrow button then you will have to press the down arrow button twice to display the next command, and vice versa with the up arrow button, when you press the down arrow key, you will have to press twice the upper arrow to get it display the right buffer.

    I also noticed that it is crashing at a point when you browse trough the buffer for a while, I think I am forgetting some part, anyone have an idea?

    Code:
    		if(wParam == VK_UP) {
    			
    			if(!Commands[ComLast]) {
    				/* No item available! */
    				SetWindowText(hWnd, "");
    				return FALSE;
    			}
    			else {
    				SetDlgItemText(GetParent(hWnd), IDC_COMMAND, Commands[ComLast]);
    			}
    
    			/* Move the retrieval index up one. */
    			ComLast = ComLast - 1;
    
    			/* Loop to end if required. */
    			if (ComLast == -1) ComLast = 15;
    		}
    		else if(wParam == VK_DOWN) {
    			if(!Commands[++ComLast]) { /* Increase here ? */
    				/* SetWindowText(hWnd, ""); */
    				/* No item available! */
    				return FALSE;
    			}
    			else {
    				SetDlgItemText(GetParent(hWnd), IDC_COMMAND, Commands[ComLast]);
    			}
    
    			if (ComLast == 16) ComLast = 0;
    		}
    This is the VK_RETURN part:
    Code:
    		if(wParam == VK_RETURN) {
    			
    			char *buf;
    
    			int buflen = GetWindowTextLength(hWnd);
    
    			if (buflen == 0) return FALSE;
    
    			if((buf = malloc(sizeof(char) * ++buflen)) == NULL) {
    				/* No Memory Space */
    				return FALSE;
    			}
    
    			/* Note: We have to get the text before we delete it. */
    			GetWindowText(hWnd, buf, buflen);
    
    			SetWindowText(hWnd, "");
    
    			/* Free the old memory pointer if it exists. */
    			if (Commands[ComIndex]) free(Commands[ComIndex]);
    
    			/* Store the buf memory pointer in the Command array. */
    			Commands[ComIndex] = buf;
    
    			/* Set retrieval slot to last stored item. */
    			ComLast = ComIndex;
    
    			/* Move storage slot to next slot in array. */
    			ComIndex++;
    
    			/* Loop to start if at end of array. */
    			if (ComIndex == 16) ComIndex = 0;
    
    			return TRUE;
    		}

  12. #12
    Hurry Slowly vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,788
    if(!Commands[++ComLast])

    isn't this prefix operation? so you increase firstly the index and then retreive the pointer - probably going out of the array bounds?

    And what about wrapping around on the actual number of entered commands - and not on hardcoded 15?
    All problems in computer science can be solved by another level of indirection,
    except for the problem of too many layers of indirection.
    – David J. Wheeler

  13. #13
    Registered User
    Join Date
    Dec 2004
    Location
    The Netherlands
    Posts
    91
    Quote Originally Posted by vart View Post
    probably going out of the array bounds
    Okay, this should fix the crash, but still, there are some problems with browsing..

    Code:
    		else if(wParam == VK_DOWN) {
    			if (++ComLast == 16) ComLast = 0; /* Check if it has reached 16 */
    
    			if(!Commands[ComLast]) {
    				/* No item available! */
    				/* SetWindowText(hWnd, ""); */
    				return FALSE;
    			}
    			else {
    				SetWindowText(hWnd, Commands[ComLast]);
    			}
    		}

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. browsing through history commands
    By apsync in forum C Programming
    Replies: 9
    Last Post: 05-22-2007, 04:54 AM
  2. Replies: 2
    Last Post: 08-25-2005, 03:09 PM
  3. best way to search through 'history'
    By anykey in forum C++ Programming
    Replies: 2
    Last Post: 08-16-2005, 04:31 PM
  4. editbox problem
    By tyouk in forum Windows Programming
    Replies: 7
    Last Post: 10-05-2004, 09:14 PM
  5. History of the Napkin
    By DavidP in forum A Brief History of Cprogramming.com
    Replies: 2
    Last Post: 01-20-2004, 01:41 AM