This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 181644 - [69cat] Wrongly Expanded Tabs in Expanded Templates
Summary: [69cat] Wrongly Expanded Tabs in Expanded Templates
Status: REOPENED
Alias: None
Product: php
Classification: Unclassified
Component: Editor (show other bugs)
Version: 6.x
Hardware: All All
: P3 normal with 2 votes (vote)
Assignee: Ondrej Brejla
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-08 15:29 UTC by laurin1
Modified: 2012-04-24 13:35 UTC (History)
4 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description laurin1 2010-03-08 15:29:26 UTC
I'm using Build 201003081350 and the tabs contained in my templates do not work. They work fine in 6.8, but in this build, they do not work at all. Once the template is expanded, there are no tabs at all. Every tab appears to be replaced with a single space.
Comment 1 laurin1 2010-03-10 08:11:15 UTC
This now happens on use of AutoComplete or Templates. Same exception.
Comment 2 Michel Graciano 2010-03-10 08:13:14 UTC
You say that some exception is thrown, if it is true please show the exeception here.
Comment 3 laurin1 2010-03-10 08:16:45 UTC
Sorry, I'm getting confused. Darnit! These are two different issues. The Exception from Templates is this bug:

https://netbeans.org/bugzilla/show_bug.cgi?id=181588

This issue is just no tabs in the templates. Though, I suspect, based on the fault, that these two are related. Before, I was just getting no tabs, now I am getting an exception. Should these bugs be combined?
Comment 4 Jiri Kovalsky 2010-03-10 08:18:35 UTC
Petre, can you please fix this? This bug is a showstopper for one NetCAT 6.9 participant and avoids him to continue. Thanks a lot!
Comment 5 Michel Graciano 2010-03-10 08:30:13 UTC
> This issue is just no tabs in the templates. Though, I suspect, based on the
> fault, that these two are related. Before, I was just getting no tabs, now I am
> getting an exception. Should these bugs be combined?

No, each problem you face you should file different bugs and if this is related developer will mark one of them as duplicated.
Comment 6 Filip Zamboj 2010-03-10 08:36:00 UTC
(In reply to comment #5)
> > This issue is just no tabs in the templates. Though, I suspect, based on the
> > fault, that these two are related. Before, I was just getting no tabs, now I am
> > getting an exception. Should these bugs be combined?
> 
> No, each problem you face you should file different bugs and if this is related
> developer will mark one of them as duplicated.

As Michael commented - this is not duplicate but may be the same reason. Keep this separate issue unless developer resolves this as duplicate. thanks.
Comment 7 Filip Zamboj 2010-03-10 08:38:10 UTC
Exception issue is filed as http://netbeans.org/bugzilla/show_bug.cgi?id=181588
Comment 8 Filip Zamboj 2010-03-10 08:54:30 UTC
Product Version: NetBeans IDE Dev (Build 100310-9ff6c4eb76b6)
Java: 1.6.0_16; Java HotSpot(TM) 64-Bit Server VM 14.2-b01

custom php template expended with tabs in my case. 
expanded text in template:
<?
$a = "this is my template" . "with tabs"; 
foreach ($array as $k => $v) {
    echo "foreach"; 
    echo $a; 
    while (true) {
        echo "hi"; 
    }
}
?>

after expansion in editor: 
<?
$a = "this is my template" . "with tabs"; 
foreach ($array as $k => $v) {
    echo "foreach"; 
    echo $a; 
    while (true) {
        echo "hi"; 
    }
}
?>


@reporter/reporters: could you submit some example? expected result and result you get if wrong? 

thanks.
Comment 9 laurin1 2010-03-10 09:29:25 UTC
That is not what I get. I can't reproduce right now, because of 

https://netbeans.org/bugzilla/show_bug.cgi?id=181588

Now, I just get an exception, but before, I was getting no tabs in my templates.
Comment 10 Petr Pisl 2010-03-10 10:33:54 UTC
The issue 181588 is now fixed. The fix should be in the next continual build and in todays night build. Could you try it then? Thanks.
Comment 11 laurin1 2010-03-10 10:36:08 UTC
I can try the next nightly. What is the continual build?
Comment 12 Michel Graciano 2010-03-10 10:40:29 UTC
(In reply to comment #11)
> What is the continual build?
All builds [1] and here [2] the main repo.

[1] http://deadlock.netbeans.org/hudson/
[2] http://deadlock.netbeans.org/hudson/job/trunk/
Comment 13 Filip Zamboj 2010-03-11 03:33:47 UTC
(In reply to comment #12)
> (In reply to comment #11)
> > What is the continual build?
> All builds [1] and here [2] the main repo.
> 
> [1] http://deadlock.netbeans.org/hudson/
> [2] http://deadlock.netbeans.org/hudson/job/trunk/

or you can use build from bertram http://bertram.netbeans.org/hudson/job/PHP-build/. that's build from repository we use and contains latest changes. These changes are propagated to other repositories then ... so main-silver contains changes a bit later than build on bertram
Comment 14 laurin1 2010-03-12 08:57:26 UTC
Ok, I just tried it in the latest Hudson build and is not working.

Here is my template:

$oTable                = new Table();
$oTable->sTableStyle    = "";
$oTable->sTableClass    = "";
$oTable->iBorder        = ;
$oTable->iCellPadding    = ;
$oTable->iCellSpacing    = ;
$oTable->Cell();

here is the result from typing table and pressing tab:

$oTable = new Table();
$oTable->sTableStyle = "";
$oTable->sTableClass = "";
$oTable->iBorder = ;
$oTable->iCellPadding = ;
$oTable->iCellSpacing = ;
$oTable->Cell();
Comment 15 Filip Zamboj 2010-03-19 10:49:01 UTC
confirmed in 
Product Version: NetBeans IDE Dev (Build 100318-3668deab869d)
Java: 1.6.0_16; Java HotSpot(TM) 64-Bit Server VM 14.2-b01

template contains syntax errors. However, after I get rid of syntax errors the issue remains 

I used template from Laurin's example formatted in the way below where spaces between variable and '=' are tabs.  

$oTable                  = new Table();
$oTable->sTableStyle     = "";
$oTable->sTableClass     = "";
$oTable->iBorder         = ;
$oTable->iCellPadding    = ;
$oTable->iCellSpacing    = ;
$oTable->Cell();


the result was without tabs: 
$oTable = new Table();
$oTable->sTableStyle = "";
$oTable->sTableClass = "";
$oTable->iBorder = "";
$oTable->iCellPadding = "";
$oTable->iCellSpacing = "";
$oTable->Cell();

I don't really think this is P2 but would be nice to have this fixed in 6.9 with respect to this is netcat report. => P3
Comment 16 Filip Zamboj 2010-03-19 10:51:59 UTC
(In reply to comment #15)
> confirmed in 
> Product Version: NetBeans IDE Dev (Build 100318-3668deab869d)
> Java: 1.6.0_16; Java HotSpot(TM) 64-Bit Server VM 14.2-b01
> 
> template contains syntax errors. However, after I get rid of syntax errors the
> issue remains 
> 
> I used template from Laurin's example formatted in the way below where spaces
> between variable and '=' are tabs.  
> 
> $oTable                  = new Table();
> $oTable->sTableStyle     = "";
> $oTable->sTableClass     = "";
> $oTable->iBorder         = ;
> $oTable->iCellPadding    = ;
> $oTable->iCellSpacing    = ;
> $oTable->Cell();
> 
one more think I added "" to template where needed to get rid of syntax errors and verify it's not due them. 
> 
> the result was without tabs: 
> $oTable = new Table();
> $oTable->sTableStyle = "";
> $oTable->sTableClass = "";
> $oTable->iBorder = "";
> $oTable->iCellPadding = "";
> $oTable->iCellSpacing = "";
> $oTable->Cell();
> 
> I don't really think this is P2 but would be nice to have this fixed in 6.9
> with respect to this is netcat report. => P3
Comment 17 laurin1 2010-03-19 12:51:48 UTC
What syntax errors?

Why is this not a P2? It was working fine up to 6.8 and now it's broke. It's very time consuming to have to go back an re-tab all of my templates.
Comment 18 laurin1 2010-03-22 13:18:51 UTC
Oh, do you mean the 

$oTable->iBorder = ;

but iBorder is a number not a string. How is that a syntax error? I don't want quotes.
Comment 19 Filip Zamboj 2010-03-22 16:19:57 UTC
(In reply to comment #18)
> Oh, do you mean the 
> 
> $oTable->iBorder = ;
> 
> but iBorder is a number not a string. How is that a syntax error? I don't want
> quotes.

the template is ok. i just checked if formatter doesn't have problems with error.
Comment 20 laurin1 2010-05-03 15:22:56 UTC
This is a regression. Why is not being addressed?
Comment 21 Petr Pisl 2010-05-04 09:43:12 UTC
I'm sorry, the reason why I haven't work on this yet, it is a lot of work and no time.

It works in previous versions due to that the formatter doesn't care about spaces at all. The current formatter format spaces according the rules. So now when your template is formated, the rule for the whitespace around equal operator is applied. The rule allows define, whether there is one space or not. Unfortunately it doesn't cover your case and the whitespace is trimmed according the definition.

Basically I don't know how to fix it. The easiest solution is that the whitespace around equal operator is not touched during inserting a templates. This solve your issue, but doesn't work correctly for other templates. So this is not the correct solution. IMHO the correct solution should be to add a rule that will cover this case. But I'm not sure how such rule should be defined. Does someone have an idea?
Comment 22 Jiri Kovalsky 2010-05-04 11:43:15 UTC
Petr, don't you want to ask Marek Fukala? This functionality works like a charm in HTML editor.
Comment 23 Jiri Kovalsky 2010-05-04 11:45:30 UTC
By the "functionality" I mean expanding abbreviation to a code template with tabs. Not tested with equal operator though.
Comment 24 Petr Pisl 2010-05-04 12:41:06 UTC
The html templates are not formatted, they are only indented. The html formatter doesn't care about whitespaces like the old php formatter. Now the php formatter is like java formatter, you can specify more then 80 rules. And the java formatter has the same issue.
Comment 25 laurin1 2010-05-04 15:36:11 UTC
I understand Petr. I'm not a Java guy, but I'm a pretty good developer. I can't really what the problem is from your description, but if I could see the relevant code, I still might be able to figure it out and I don't mind putting in the time to do it. Can you send it to me or point me to the quickest way to look at it?
Comment 26 Petr Pisl 2010-05-04 16:25:09 UTC
Laurin1, thanks for the offer. But the problem is not in the code. I will try to explain it in more details. 

Every template is formatted before is inserted to a document. It's formatted by the same formatter that formats your code after Format Action (ALT+SHIFT+F). Sure, there is not formatted whole file, but it's called format action for the selection, where the selection is the template. So when you put your template by hand to a document, then select the text of the template and call the format action for this selection, you should get the same result like during inserting the template through abbreviation. The previous PHP formatter (the formatter in NB 6.5 - 6.8) wasn't really formatter but "indenter". This old formatter only corrects whitespaces at the beginning of the lines. So the spaces before equal operator in your template were not changed by formatting, because the functionality wasn't here.

The new formatter is completely rewritten and its behavior is defined according the rules, that can be defined by a user in formatting options. If you go to Options -> Editor -> Formatting and choose PHP language, then in the Spaces category you can define, whether there should be space around equal operator. So when the new formatter find the equal operator, then creates spaces according the mentioned rule.

But the rule is saying, whether there will be one space before and after the equal operator or non space. If we want to keep the possibility to format whitespaces around equal operator, the rule has to be somehow extended in the UI and in the algorithm as well.

Let's say that we will add a new options something like : Align equal operator . Probably not good wording. But we don't want align all equals operator in the file during formatting whole file. So there has to be defined, for which lines the operator will be aligned. 

If you want to look through the code you can see it here: http://hg.netbeans.org/web-main/file/b56f0bb24b1f/php.editor/src/org/netbeans/modules/php/editor/indent

The files, that influence the formatter:
FmtOption - load, save format options 
FormatToken - defines token for the formatter
FormatVisitor - creates list of format tokens
TokenFormatter - this file is the most important. There are applied the rules over the list of format tokens. 

The algorithm is:

FormatVisitor creates a list of tokens based on AST (result of our PHP parser) combined with Lexical analysis (Lexer).
Token formatter takes this list and applies the rules over the list of format tokens. Usually one whitespace is not influence only by one rule. I'm not sure whether you want to go through the code, it's a lot of code and I have more then 340 tests for the formatter now. 

There were two reasons why we decided to rewrite formatter.
1) The previous implementation didn't allow to define complex options, like alignment or wrapping.
2) The previous implementation had performance problems. The formatting of tcpdf.php file took on my computer more then 2 minutes (finding format changes 26 second). With the new implementation it takes less minute (finding format changes 400 ms).
Comment 27 Marek Fukala 2010-05-04 16:46:23 UTC
(In reply to comment #22)
> Petr, don't you want to ask Marek Fukala? This functionality works like a charm
> in HTML editor.

That's nice from you since you present the inability of the html editor properly format the code as a feature :-). 

Seriously, my personal opinion is that the abbreviations should be formatted according to the default IDE rules when inserted into the code. So I fully agree with Petr Pisl that this is not a regression, not even a bug. That is just a changed behaviour compared to previous errorneous state. BTW the code will be adjusted anyway after the first file reformat.

Another aspect of this is that there could be a formatting rule-option one could use to adjust the precise behaviour he expects. But this is an enhancement.
Comment 28 laurin1 2010-05-04 16:50:41 UTC
I know I am going to get rocks thrown at me, but I have to ask this question: Why? 

Why are we formatting templates? I mean, the formatter is an option, which we do not use, because it does not format the way we like our code. So, if I want to use templates, I don't get the option.
Comment 29 Marek Fukala 2010-05-04 18:29:25 UTC
I belive the templates are formatted because we belive/assume that the code is formatted correctly. I understand that it can become pretty annoying if it's not true for you. Then I would file an issue against the formatter, not against the fact that the abbreviations are formatted by default.

This is just my opinion. I understand that something that seems to be really a marginal problem to us may be a major obstacle for others. Maybe Petr can create some undocumented option for you ...
Comment 30 Petr Pisl 2010-05-05 14:51:40 UTC
The templates are formatted, because then the text follow the rest of code. For example templates for foreach cycle and other templates that have { } inside. The placement of { and } is done according the formatter rules. 

Unfortunately it doesn't fit your case. We can ask guys from editor infrastructure to introduce an option, which will say, whether the template should be formatted or not, but as Marek mentioned, when you invoke format action after this, it will be changed anyway.

IMHO there is only one correct solution, to create a formatting rule that will cover this case. And this rule will not be simple for implementation:(.
Comment 31 Petr Pisl 2010-05-17 14:43:51 UTC
I'm marking as enhancement, because it can be satisfied, only when new rules will be added to the formatter as I described above.
Comment 32 laurin1 2010-07-09 14:03:26 UTC
I have another problem with the current status of this "bug/enhancement".

It's not even formatting my templates per my formatting settings. Look at this piece of a template:

	public static function getInstance(){

		if(!self::$instance instanceof self) self::$instance = new self;

		return self::$instance;

	}

This is how it gets formatted:

	public static function getInstance(){

		if( ! self::$instance instanceof self)
			self::$instance = new self;

		return self::$instance;
		}

Based on my formatting rules, it should be formatted like this:

	public static function getInstance(){

		if(!self::$instance instanceof self)
			self::$instance = new self;

		return self::$instance;
		}

I have it configured for NO SPACES within parentheses. Not sure about after the !. What setting is that?