Bug#557135

The patch for indent went through a lot of iterations and changes. I have summarized everything up until this point as to where I am with my latest patch:

- Selected lines are indented as well as multiple lines.
- Indentation on multiple lines is grouped into a single undo/redo event.
- Rather then replacing the entire line with <indention> + <line text> only indentations are inserted this time.
- After indenting if there is a selection, the selection is retained.
- Indentation tab size on multiple lines is now consistent
- When undoing the selection retains its expected position.

After Julian reviewed my final patch he had recommended me the following changes before he could push the patch.

1.) As of now the code is a bit complicated to read and has to be structured properly.

2.) Be smart about leading spaces to determine how many tab spaces are required to align a text. An example Julian provided:

For instance if tabstop = 4 and current line follows with cursor position at col 1 :

“space|spaceHello”

After hitting “tab” it would add 3 spaces since we’re using cursor position to determine how many spaces are needed.

“spacespacespacespace|spaceHello”

Now we have 5 spaces in front of Hello. Instead there should be only 4 because we need to take the leading spaces into accountability and therefore only 2 spaces are required to align “Hello”:

“spacespacespace|spaceHello”

It is clear to me as to what needs to be done and how to do it. I will have my final patch before the end of this week.

Cheers!

After I shortly submitted my 2nd patch, Julian was able to provide me with feedback and as well as a new patch with a few more additions to my current patch. My patch worked  but however there were a few issues pointed out by Julian that were needed to be fixed and are fixed now with his help:)

1.) Instead of replacing the entire line with <indention> + < line text > , only indentations are inserted this time.

2.) Undo/Redo - Thanks to Julian for the code, indentation on multiple lines is now grouped into one undo/redo event.

3.) Since ‘count’ always retained the same value in the ‘line for loop’ it was not necessary for it to be executed more then once. I moved it outside of the ‘line for loop’ 

4.) After a selection was indented, the selected text was no longer selected. Now the selected text remains selected.

I have submitted a patch on bugzilla Bug#557135 with these changes. 
*source code: http://gist.github.com/366757

More to follow……

Well before I get straight to the multi-line stuff, let me mention a few bugs I encountered while working on this feature. 

1.) Undo/Redo did not work properly with multi-line indent. Each indent on a line was treated as a separate change where as in the whole indent should be grouped into one undo event. Julian is aware of this now. I might know how to fix this so If I do come across a fix I will do it:)

2.) Skipping Tab Spaces I noticed that when moving from left to right if there is a tab space it does not skip a tab worth of spaces when it should. Derek is working on this.

After Julian reviewed my code, he told me that I should not be hard coding the tab spaces and instead I should use ‘tabstop’ to determine the amount of spaces I need. Apart from that everything else looked good. I altered my code and below are the changes: 

Complete source code: http://gist.github.com/363245

BTW It is Bug#557135

Minor Update* 
If the total tab spaces for every other line will always be the same then I believe the code below should go right after “selection.start.column = 0;” and before the for loop begins.  

I will wait for Julian to provide me feedback and go from there:)

Schools just a week away from finishing and I have been working on Bespin. In my previous post I was able to get indent working on a selected piece of text. Now Julian had suggested that I change the code so that it would work on multiple lines. 

First step was to grab all the selected lines:

*This gives us an array of selected lines which now contain the characters and the length of each line.

Great so we have the lines we would like to indent! Now next step was to iterate through each line and add an indent space in front of it. But of course the first line was special on its own since its start position could be selected from any column, unlike the lines afterward which would always retain their start position at column 0. 

In order for me to add the indentation to any given line I would have to call the function “replaceCharacters(range,text_to_be_replaced_with)”. So therefore I would need to specify a range object which would point to that specific line. Here is how a range object looks like:

range = { 
                start: { row: 0, column: 13}, 
                end:   { row: 1, column: 24}
} 

“start” position is where the selection starts and “end” is where the selection finishes. They both have “row” which specifies the line number and “column” which represents at which character. 

Now since its a bit more clear let me show you my complete code..I’ll start from chunks:

@line 15, we will add indentation to the first line, and since “selection” already points to the first line we do not need to modify it. 

Now we loop through each line and add 8 spaces of indentation. “current” identifies the row position where the first line is, since its not always 0!. Rest is self explanatory.

After the indentations have been added where needed, we need to reposition the cursor. 

Here is the code

Complete source code:http://pastebin.com/Vi4RgscU

For now It does indent multiple lines, However it is not the final code. I will revisit the code and make a few minor modifications. I do have question regarding the tab spaces but I will get Julian to review my code. I doubt its 100% right but I am getting there…I’ll post more updates until then have a great weekend!

p.s When I got the new repository I realized the code was not in the text.js file anymore, it had been moved to a different file named editing.js under “commands” folder.. 

In the previous version of Bespin you were able to highlight a piece of text and upon hitting the “tab” key it would shift the text to the right and cause it to indent. However this feature is not available in the most recent version  0.7.1. Julian asked me to bring this feature back.

I started my work in the textview.js ; Where the code would belong for this type of action. I Looked for the function that got triggered when the “tab” key was pressed.

source:http://pastebin.com/bjTd8K0E

The original code adds the tab space but overwrites the selected text while doing so.  To avoid this type of behaviour I appended the highlighted text (this.getSelectedCharacters() returns the highlighted characters) along with the tab space(str). This did the trick!

However while working on this bug I noticed a strange behaviour, every time you would hit the “tab” key, the focus went to the command line, which was very annoying. I had a feeling it was a bug and asked on #Bespin IRC channel. Patrick told me that it was Bug#555598 . After viewing the bug I noticed a patch had just been submitted, It just needed to be pulled.  Next up is “Unindent”…um yeah at first I was like wth…Its when you hold shift + tab and everything gets indented to the left(notice the two arrows on your tab key:D).

I usually wait to blog when I have accomplished a task or I believe I have something good to say. It’s neither. When I had started to work on my bespin word wrap bug I was not sure where to start. Normally when I start my work on any programming assignment I usually go straight to coding and try to experiment with the code and start narrowing it down until I get something right going. Of course this is not the best step if your working with a large code base. Then again at the same time you need to have some clue as to where you “might” think the code resides using keywords or just simple guessing loll?

I went into the file text.js because I had done some work in there previously. I began playing around and I was able to find the code where it inserts the character onto the line but it was not enough to do what I wanted. 

I needed help so the first thing I did was, I got on the IRC #Bespin channel and informed about my bug and that I needed help. Instantly Julian Viereck replied and told me to post my thoughts and suggestions on Bespin-core mailing list. I did and I got a few replies. First suggestion was by Dany Zatuchna for me to use the “Knuth’s Algorithm”. I took a look but it was not exactly what I needed at the moment. I wanted to break the solution into a few steps: 

1.) Get the length of each line
2.) Find out the max width of the editor
3.) Set a speciific margin so if the user reaches the end of a line then insert the      characters on to the next line.
4.) From there I can do the word wrap which would be iterating through each line and swapping words where necessary. 

Kevin Dangoor had told me that the editor lives in the ScrollView and that’s how it knows its dimensions. And that it uses character’s width and height to determine the total area of the editor. Interestingly It was not what I thought it would be. He also Implied that I should talk to Patrick and Julian. Since both had done most of the hacking in bespin-core. After going back to the IRC channel and speaking to Patrick Walton, I found out that most of my hacking would be done in LayoutManager and a little modification in the TextView. LayoutManager because it mostly entails what word wrap require. It is able to determine the pixel position of each character or a range within the editor.

However after spending some time with both files I was still unsure as to where I would start coding. Also I did not understand some of the functions in the file LayoutManager. So I sent Julian an email about my questions and he felt that this bug was not simple and it would be hard to do if I hadn’t had much experience working with Bespin coding. He offered to guide me through it but felt that I should work on the bugs suggested by him first so that I could get more fimiliar with the Bespin code base.

At the end of the day there are no stupid questions and from every question you learn something even if you don’t accomplish anything. I made a mistake and that was not being open about my problem. The idea of me looking “stupid” by not being able to do this or asking “stupid” questions kept stressing me. I was wrong and it’s not like It was not obvious to me. You only get better at something by keep working on it….Learning can come from experimenting and asking questions. Not everyone has the answer to everything… At the same time you might know something someone else might not and thats how we collaborate.

Now having said that I got a better understanding of how the Bespin code structure is layed out, similar to the way Cocoa does. In the mean time I’ll work on other bugs recommended by Julian Viereck. Then I will come back and experiment this word wrap bug.

My second prototype draft for the file system for our PJS IDE. This includes the basic functionalities such as creating a new file, opening an existing file, deleting files/folders and moving them.Any feedback or suggestions are welcome. I will get  my team’s input and suggestions and from there we will start making our revisions.

It seems like hours, days, and months…since I last worked on a bug. My last release was for Bug #509492.  A few days ago I acquired myself a new Bug #542524. So What is the problem? Not necessary a problem but a feature.The folks at Bespin want to implement word wrap feature into the Bespin Editor. Now what the heck is word wrap? Word wrap is a feature that allows an editor to automatically shift words to the next line if the words are too large to fit onto the current line.

First step

Thanks to David Humphrey for pointing out how I should get started with this. Initially I began reading about various Algorithms used to perform word wrap and the most important factor is optimization. As much as we love to get a piece of code to do what we want it do, we do have to ask our self is it the most efficient way ? Even after speaking to Kevin Dangoor, his only requirement was to make sure that this feature did not slow down the editor!

Greedy Algorithm

SpaceLeft := LineWidth

for each Word in Text

    if Width(Word) > SpaceLeft

        insert line break before Word in Text

        SpaceLeft := LineWidth - Width(Word)

    else

        SpaceLeft := SpaceLeft - (Width(Word) + SpaceWidth)

This Algorithm is called the “Greedy Algorithm” and it used by popular word processors like Microsoft Word and Open Office. This pseudo code as described goes through each word and tries to fit as many words it can on a line and continues this routine by going to the next line until there are no words left to insert.

Getting Started…?

So I get the idea of how to do this but I’m unclear as to where the code would go, I’m guessing it should be a function that should called when the editor is redrawn after each character is typed. I will ask the folks on #Bespin IRC channel for help. 

In the mean time I will start playing around with the code but I’m very hungry and I need to have my dinner…So I’ll continue from here, bye:) 

So Let me present you Part 2 of this Thunderbird Lab and no the caffeine is not kicking in, I’m trying to avoid any intake of coffee… After the initial debug build, I tried to reproduce the issue to ensure that the problem persisted in order for me to fix it in the first place.

Reproducing The Problem

This is a draft copy of an email I sent to myself and as you can see that all the current invalid email addresses contain two or more “.” after the @.

Fixing The Code

To resolve this and thanks to Scott for pointing out “Find()” function belonging to the nsDependentString class; It allows us to check for a specific pattern in a string starting at any given index, and if the pattern is found it will return the position where the pattern first occurred else it will return “-1” meaning not found.

if (inString.FindChar(‘.’, pos) != kNotFound)
This is the original code and it only checks for a single ‘.’  after ‘pos’ which is the index of “@”….So to check for two ‘..’ I appended this to the current code

if (inString.FindChar(‘.’, pos) != kNotFound) && !inString.Find(“..”,pos))
Now the code simply says in the string “inString” look for a single “.” and not “..” and if both conditions are met then a valid email address is found and create the string literal as a “mailto” link.

Result

Seems like the code fixed the problem.

Bye

Its been ages since I have blogged but I’m back on it !..Currently we are working on a Thunderbird Lab, which requires us to fix a bug regarding unwanted mailto link. This Lab will provide us a practical understanding of how to file a bug, finding a fix for it, create a patch for the fix and then submitting the patch for a review.
The Problem?

Even though “S@Y…” is a string literal in the message it still detects it as a valid email address and that definitely is not a valid email address! The problem in this scenario is that there should only be a single period between the character ‘Y’ followed by ‘@’ and domain name ‘where’ for it to be considered valid. Before resolving the problem, I decided to do a debug build of Thunderbird.

For instructions on how to build Thunderbird, visit  Mozilla Developer Center website and locate the article “Simple Thunderbird build”. I’m building this on my windows laptop.

1.) Retrieve the source code for Thunderbird using hg clone http://hg.mozilla.org/comm-central/

2.)cd comm-central

3.) Configure .mozconfig file for a debug build:

mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/objdir-tb-debug

ac_add_options --enable-application=mail

ac_add_options --enable-debug

ac_add_options --disable-optimize

4.) python client.py checkout
5.) make -f client.mk(Coffee break indeed)

I successfully builded Thunderbird but with an error regarding NSGlue_Assesrtion..:(:(Seems like Than Dao and Scott Downe encountered this error as well! Anyways I’m off to bed I’ll have to check that tomorrow and continue from there.

Gnite!