Jump to content

General Question About Tasks


Recommended Posts

This is a question to the dev leaders and probably of general interest to all new developers. I just finished work on a task and had a few questions about how I completed the it.

In this case, I added some code to CInput to handle word delete keystrokes - this was the scope of the task request. Widget code is tricky stuff, so since I was already in there and I had added the logic for discovering word boundaries and was familiar with the code, I also added the code to handle moving around a word at a time. And since I was already in there and doing word stuff, I added the code to handle word selection via. mouse double clicks. Then I noticed that there was code for paste, but no code for copy and cut, so I added them as well. When I tested the cut/paste/copy stuff I discovered a really hard to find bug that made intra-application clipboard operations on Windows fail. So I fixed that bug.

My question is, given the above scenario, how would you prefer me to handle such situations? Would you prefer that I would have stopped at some point and created new tasks? If so where?

I did finally close the task (well changed it to review) and created a couple of new tasks for CInput because the patch was getting fairly large. Please let me know your preferences on such things.

Cheers!

Link to comment
Share on other sites

Great job on that ticket! I have just reviewed, cleaned up and committed the patch.

These features are very useful; thanks for adding them!

I think your approach of adding similar features and fixing a bug that came up makes sense. There is no benefit to generating extra "paperwork" if the other tasks are related and you are willing and able to accomplish them in one sitting. I assume you have a local todo-list that provides a little morale boost as each entry is marked done, so there's no need to use trac tickets for that.

However, the two extra tickets you created make sense, since they relate to different topics and might take longer to fix.

As to patch size - we've had huge ones before; if that's called for by the size/scope of the task, so be it. It's good keep each patch related to a single issue, though, so we don't accidentally throw the baby out with the bathwater and/or get things confused :)

Link to comment
Share on other sites

Ok Jan, thanks for the feedback. Sounds like we are in sync.

I noticed you found a window handle that worked with the clipboard, glad to see that! Also, I noticed how you cleaned up the error handling in wclipboard. A lot of the older code is coded similar to the way wclipboard was (little or no error checking returning nums or zero instead of symbolic constants), so I will follow your lead and make similar changes when I run into that type of code.

Cheers

Link to comment
Share on other sites

Oh yeah. Some of the code is quite old (mentioning problems with Win98 and 2000), and some of it was also designed to minimize size (being part of 4K demos).

That's great, similar cleanups are very welcome :)

Additionally, two minor requests: please use tabs (not spaces) and try to match the coding convention at that particular spot. Certain parts use if (expression), others write if(). It's nice to keep that consistent.

Link to comment
Share on other sites

Additionally ... please use tabs (not spaces) ...

Hi Jan, I thought I had keep tabs selected in VS2010, I will verify that before I make any more changes.

Cheers

Hi Jan, I verified that I have the use tabs turned on. I usually use spaces, so its possible I'm used to typing in spaces and that is where they originated ;). Anyway, I will be more careful about that now.

Edited by Chakakhan
Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share

×
×
  • Create New...