Skip to content

Conversation

@tulinkry
Copy link
Contributor

fixes #1627

@tulinkry tulinkry added this to the 1.1 milestone Jul 17, 2017
@tulinkry tulinkry self-assigned this Jul 17, 2017
@tarzanek
Copy link
Contributor

I think you just miss a fix for unit tests ... otherwise it looks OK

@tulinkry
Copy link
Contributor Author

I adapted the tests by turning off scopes and I added one tests where scopes are enabled

@vladak
Copy link
Member

vladak commented Jul 21, 2017

From maintenance perspective I don't like the addition of verbatim HTML code too much. Ideally it should be parsed to be get the scopes.

@tulinkry
Copy link
Contributor Author

For test purpose this seems ok to me now, taken into account that I only followed the other test case in this package. Of course a kind of less strict test would be welcomed.

lxref.reInit(in);
lxref.annotation = annotation;
lxref.project = project;
lxref.setScopesEnabled(RuntimeEnvironment.getInstance().isScopesEnabled());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I understand this and AnalyzerGuru.java#writeXref() are used only for testing. Otherwise during normal indexing the xref is created via this path:

IndexDatabase#addFile()
  FileAnalyzer fa = AnalyzerGuru.getAnalyzer() // fa can be e.g. CAnalyzer
  AnalyzerGuru#populateDocument()
    fa.analyze()
    AbstractSourceCodeAnalyzer#analyze()
      super.analyze()
        PlainAnalyzer#analyze()
          writeXref()
            xref = newXref() // xref can be e.g. CXref
            JFlexXref.write()

where the scopes/folding properties are in effect thanks to the inheritance and these lines in IndexDatabase#addFile():

662          fa.setScopesEnabled(RuntimeEnvironment.getInstance().isScopesEnabled());
663          fa.setFoldingEnabled(RuntimeEnvironment.getInstance().isFoldingEnabled());

So having these properties set into the writeXref() seems like short-circuiting however there is no other way given the method is static.

The only nit I have is that the call to reInit() in JFlexXref above contains:

        scopes = new Scopes();

which is not necessary when scopes are disabled. Granted, this method does not do anything however it's a bit of wasted memory.


protected void startScope() {
if (scopesEnabled && scope == null) {
if (scopesEnabled && scope == null && defs != null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this fixing any misbehavior ?

@tarzanek tarzanek modified the milestones: 1.1, 1.2 Jan 10, 2019
@tulinkry tulinkry closed this Jun 21, 2019
@tulinkry tulinkry deleted the annotations-with-scopes branch June 21, 2019 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Scopes lost in Annotate view

3 participants