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 228645 - clarify the usecases for CPCustomIndexer/CPIndex [was:CPIndex sometimes returns no files]
Summary: clarify the usecases for CPCustomIndexer/CPIndex [was:CPIndex sometimes retur...
Status: RESOLVED FIXED
Alias: None
Product: web
Classification: Unclassified
Component: CSS Editor (show other bugs)
Version: 7.4
Hardware: All All
: P2 normal (vote)
Assignee: Marek Fukala
URL:
Keywords: RANDOM
Depends on: 228650
Blocks:
  Show dependency tree
 
Reported: 2013-04-17 08:44 UTC by Tomas Mysik
Modified: 2013-06-06 01:51 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
Exception Reporter:


Attachments
Patch with info messages in console (1.65 KB, patch)
2013-04-17 08:44 UTC, Tomas Mysik
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Mysik 2013-04-17 08:44:21 UTC
Created attachment 133537 [details]
Patch with info messages in console

This issue is random, to reproduce it you can follow steps from issue #228608. Moreover I am even editing my LESS file in the editor and the output in the console (with the attached patched applied) is:

INFO [org.netbeans.modules.css.prep.process.BaseProcessor]: Not compiling, no mappings for project /home/gapon/NetBeansProjects/HTML5Application12
INFO [CPCustomIndexer]: File newstyle.scss marked as CSS preprocessor file with text/scss mimetype.
------------------ indexing running: false
------------------ SASS files: []
------------------ indexing running: false
------------------ LESS files: []

So the file is correctly recognized by the CPIndex but no Sass/LESS files are returned from index (and idexer is currently not running).

Thanks.

Product Version: NetBeans IDE Dev (Build 20130416-d5672768d1c8)
Java: 1.7.0_17; Java HotSpot(TM) 64-Bit Server VM 23.7-b01
Runtime: Java(TM) SE Runtime Environment 1.7.0_17-b02
System: Linux version 3.5.0-27-generic running on amd64; UTF-8; cs_CZ (nb)
Comment 1 Marek Fukala 2013-04-17 11:59:08 UTC
As Tomas Zezula is going to implement #228650 and Tomas Mysik will then likely replace the CPCustomIndexer/CPIndex by the new API there's no need to fix this right now. BTW it looks like some indexing issue anyway.
Comment 2 Marek Fukala 2013-04-17 12:02:03 UTC
BTW As I'm not sure when the issue 228650 is going to be implemented, I better ask once again: Is THIS issue critical? Do we need this to be fixed in close future? If so I may take a look at it ...
Comment 3 Tomas Mysik 2013-04-17 12:31:58 UTC
(In reply to comment #2)
> BTW As I'm not sure when the issue 228650 is going to be implemented, I better
> ask once again: Is THIS issue critical? Do we need this to be fixed in close
> future? If so I may take a look at it ...

Petře?
Comment 4 Petr Jiricka 2013-04-17 13:22:49 UTC
I have no idea how important this is, I did not file it. Is there any practical impact on the user? Seems that you were able to fix bug 228608 (which I filed) independently of this problem.
Comment 5 Tomas Mysik 2013-04-17 13:26:03 UTC
The point is that even if I fixed issue #228608, this issue causes the very same problem (project problems does not appear sometimes if you add the first Sass/LESS file so the same steps as in your issue).
Comment 6 David Konecny 2013-04-18 20:36:04 UTC
I would think this issue is critical as css processors feature depends on it and consequently cannot work properly without it.
Comment 7 Petr Jiricka 2013-04-24 13:59:24 UTC
I agree, I also hit this problem (I assume), my steps are:

1. Make sure SASS compiler is set in IDE options
2. Unpack and open the project from http://wiki.netbeans.org/wiki/images/e/ee/SourceMapsExample.tar.gz

=> I would expect the IDE to show a project warning as the output mapping is not set, but there is no warning on the project.
Comment 8 Tomas Mysik 2013-05-30 05:04:04 UTC
This issue is likely WONTFIX; see issue #230445 for more information (we don't heed this functionality anymore so the whole indexer can be removed). I will update this issue once we have final decision.

Thanks.
Comment 9 Tomas Mysik 2013-05-30 10:06:14 UTC
Marku, we do not use CPIndex anymore - please, remove it completely. Guys on CC, please stop Marek if I am wrong :)

Thanks.
Comment 10 David Konecny 2013-05-30 20:02:57 UTC
I'd think we are still using the index to check whether all SASS files are compiled upon project opening.
Comment 11 Tomas Mysik 2013-05-31 04:44:28 UTC
(In reply to comment #10)
> I'd think we are still using the index to check whether all SASS files are
> compiled upon project opening.

No, this functionality was removed (quite long time ago). AFAIR there was an agreement that it cannot happen to have inconsistent Sass/LESS and CSS files on project opening.

Thanks.
Comment 12 David Konecny 2013-06-04 00:02:16 UTC
(In reply to comment #11)
> (In reply to comment #10)
> > I'd think we are still using the index to check whether all SASS files are
> > compiled upon project opening.
> 
> No, this functionality was removed (quite long time ago). AFAIR there was an
> agreement that it cannot happen to have inconsistent Sass/LESS and CSS files on
> project opening.

I never heard that agreement. :-) The first version of SASS compiler in trunk had a problem that all files were always recompiled upon project openning and JB's windows laptop become unusable. That was wrong.

Checking if some files are out of date makes sense to me. Either upon project open, or by providing a badge of file's icons (as we do for Java files which were not compiled) or some other means. In most of the cases it should result into no action. But in situations when user did some mistake and not all files are compiled this help from the IDE can be very handy.

I'm not pushing this feature but if we already have an indexer then I would rather keep it than delete it. And either for 7.4 or later re-introduced compilation status check.
Comment 13 David Konecny 2013-06-04 03:21:36 UTC
Another question related to this issue - how do you Tomas plan to handle case like this: user has main style.sass file which @imports _partial.sass file and user saves _partial.sass. At the moment saving partial file does not trigger recompilation of main sass file which is incorrect. But how are you going to fix it? I think you have two options: #1) parse SASS files and create a dependency graph; or #2) recompile all SASS files. Both of those solutions will require this indexer, no?
Comment 14 Tomas Mysik 2013-06-04 05:14:00 UTC
(In reply to comment #13)
> At the moment saving partial file does not trigger
> recompilation of main sass file which is incorrect.

This would be a bug, right? Or how it could happen?

> #1) parse SASS files and create a
> dependency graph;

Yes, this is what we do. Or better - I just ask Marek's API to tell me this information.

> Both of those solutions will
> require this indexer, no?

No, AFAIK there are 2 indexers - the one this issue is about is just collecting file paths of Sass/LESS files, nothing more. BTW there should not be such indexes in IDE at all, we have this functionality already in Go To File dialog - see issue #228650. But Marek is the right person to answer this question, of course.

Thanks.
Comment 15 Tomas Mysik 2013-06-04 05:29:22 UTC
(In reply to comment #12)
> I never heard that agreement. :-) The first version of SASS compiler in trunk
> had a problem that all files were always recompiled upon project openning and
> JB's windows laptop become unusable. That was wrong.

I really believe that this was discussed on Easel mailing list - I hope I am not wrong... Petře, don't you remember it, please? I can try to find it but searching in e-mails is never easy :)

> Checking if some files are out of date makes sense to me.

I already wrote it earlier - how this can happen? How can be any file out of date? The only way to get into this situation is - IMO - if one edits any Sass/LESS file, does _not_ recompile it and does _not_ check it in the browser - not usual, I would say. Also, if one uses any SCM, I think that all generated files must be in the repository, simply to avoid any differences in the CSS files. Personally, I cannot imagine not to do that - the Sass/LESS files would have to be compiled on the deployed server which would not be good idea, I think (due to bugs in compilers, everyone could have a different generated CSS file, right?).

> Either upon project
> open, or by providing a badge of file's icons (as we do for Java files which
> were not compiled) or some other means. In most of the cases it should result
> into no action. But in situations when user did some mistake and not all files
> are compiled this help from the IDE can be very handy.
> 
> I'm not pushing this feature but if we already have an indexer then I would
> rather keep it than delete it. And either for 7.4 or later re-introduced
> compilation status check.

This "path-collecting" indexer should be definitely removed and the existing Go To File API should be used, not only from the performance view. And because we do not have anything like "compilation status check" right now, I don't think we will duplicate any work.

Thanks.
Comment 16 Marek Fukala 2013-06-04 15:13:12 UTC
@Tomas: I believe you should be the one who knows the best if you need the CPCustomIndexer/CPIndex or not. I was just asked by you to create a facility which tells you if there are any CP files in a project.

Right now the only client of CPIndex is yours CssPreprocessorUtils.hasAnyFilesForCompiling() which has zero usages.

I've reassigned the issue (which complains about the CPIndexer's reliability) to you as the functionality is apparently not used. You may either rewrite it to the mentioned Tomas' go-to-source API (btw what is the code base?) or decide you really need the current implementation. In case of the first option, just delete it and as for the second just reassign the issue back to me along with some unit tests which illustrates how you plan to finally use it.

@David: As for the file dependencies, these are provided by the "plain" css index (css.editor module) and are not affected by the CPCustomIndexer/CPIndex.
Comment 17 David Konecny 2013-06-04 22:27:43 UTC
(In reply to comment #14)
> > At the moment saving partial file does not trigger
> > recompilation of main sass file which is incorrect.
> 
> This would be a bug, right? Or how it could happen?

Did you try to create a project for JET sources? I did and it does not work. I filled it as issue 230750 - it is caused by dot character in filename; JET uses dots a lot in its filenames.

> > #1) parse SASS files and create a
> > dependency graph;
> 
> Yes, this is what we do. Or better - I just ask Marek's API to tell me this
> information.

That makes sense. Thanks. Because of issue 230750 I thought this is not implemented.

(In reply to comment #15)
> I already wrote it earlier - how this can happen?

As I said it is just a consistency check to rare user's error. How it can happen? By a mistake. We do them all the time. Big team, complex project, junior developers, ... it is easy to decide to hotfix SASS color from blue to darkblue in a notepad without testing the change because it is sooo simple and save change.

But as I said, it's P3 IMO, I'm not pushing it, do what you want. :-)
Comment 18 Tomas Mysik 2013-06-05 06:40:03 UTC
OK, I will do this:

* remove this CP indexer, it should exist (reasons explained),
* submit an issue about "up-to-date" check for Sass/LESS files.

Thanks.
Comment 19 Tomas Mysik 2013-06-05 06:45:14 UTC
(In reply to comment #18)
> * submit an issue about "up-to-date" check for Sass/LESS files.

Done, issue #230757.
Comment 20 Tomas Mysik 2013-06-05 06:48:25 UTC
(In reply to comment #18)
> * remove this CP indexer, it should exist (reasons explained),

OK, since I do not want to break something, Marku, please do so.

Thanks.
Comment 21 Marek Fukala 2013-06-05 08:13:46 UTC
changeset:   255045:6333541a44d1
summary:     #228645 - removing unneeded indexer/index
Comment 22 Quality Engineering 2013-06-06 01:51:15 UTC
Integrated into 'main-golden', will be available in build *201306052301* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/6333541a44d1
User: Marek Fukala <mfukala@netbeans.org>
Log: #228645 - removing unneeded indexer/index