Skip to content

displaying a file and clicking on Annotate link fails to display annotations #1787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
shaehn opened this issue Sep 25, 2017 · 11 comments · Fixed by #1791
Closed

displaying a file and clicking on Annotate link fails to display annotations #1787

shaehn opened this issue Sep 25, 2017 · 11 comments · Fixed by #1791

Comments

@shaehn
Copy link
Contributor

shaehn commented Sep 25, 2017

I am building from the most recent OpenGrok/master. I index a set of files in an AccuRev repository. Using TomCat service to start OpenGrok. After displaying a file, I select the Annotate link for the file, but rewarded with no changes. I have put breakpoints in the AccuRevRepository.getHistory call and see that actual data is being returned, but after that I have not idea where its gone. Has anyone else had this problem recently? It was all working fine about a month ago.

@vladak
Copy link
Member

vladak commented Sep 25, 2017

do annotations work for other SCMs or just AccuRev ?

@shaehn
Copy link
Contributor Author

shaehn commented Sep 27, 2017

I am not sure what you are asking. Annotations have been implemented for all the Repositories. It is part of the Repository interface. It is supposed to show the 'blame' data on the file page. The problem seems to be somewhere in the generation of the web page, I cannot figure out where this is handled in the .jsp files. The very first time the Annotate link is poked generates the call to the individual Repository interface to get the data. Subsequent selections do nothing more that hide/show (via html class addition/removal).

@shaehn
Copy link
Contributor Author

shaehn commented Sep 27, 2017

Vlad, have you ever used the annotation feature? What I have found in subsequent experiments is that the 'A' link in a directory listing actually displays the annotated file, then the Annotate link on the page will hide/show those annotations. Is that how it works now (or has it always worked this way)? I could have sworn that the Annotate link on the file page actually loaded the annotated file before.

@vladak
Copy link
Member

vladak commented Sep 27, 2017

I use Annotations all the time. Yes, 'A' will display annotated file. Yes, Annotate link will trigger show/hide. If you display a file (not going through the 'A' link) and click on the Annotate link it will show annotations. It always worked like that.

@vladak
Copy link
Member

vladak commented Sep 27, 2017

list.jsp contains a line that starts // annotate. This is where the UI part of annotation view is done.

@vladak
Copy link
Member

vladak commented Sep 27, 2017

Just tried that on the latest master (cset 9ce9e4d) in a git repo and it is indeed broken.

@vladak vladak changed the title Unable to see Annotations on file? displaying a file and clicking on Annotate link fails to display annotations Sep 27, 2017
@vladak
Copy link
Member

vladak commented Sep 27, 2017

This might be related to the changes for #1745 and #1736 that change the URI of a file to contain the latest revision ID.

@shaehn
Copy link
Contributor Author

shaehn commented Sep 28, 2017

Thanks for verifying that I am not losing my marbles/mind :-)

So what I am seeing when I put a debugger on this is that the first call to PageConfig.getAnnotation makes its way into the method annotate and it fails to declare 'true' because of the test

   Boolean.parseBoolean(req.getParameter("a") returns a  false value.

(I am going to assume that the parameter 'a' is not present in the request). I have no idea how parameter 'a' was set originally before those changes you mentioned above.
...
After further study, definitely the changes you added in #1736 broke this ability. I obtained a copy of list.jsp prior to the changes, and the Annotate link works as expected.

@shaehn
Copy link
Contributor Author

shaehn commented Sep 28, 2017

I have a question about debugging the .jsp/.jspf files. I have noticed that when these files are 'included' the debugger doesn't seem to open them up when I am stepping through code. The Debugging window shows that the process is suspended in the included file and at a specific line, but what is being displayed is the file that is doing the 'including'. (I am using Tomcat) Is there something I need to set up to get this to work properly, that is display the file where the suspension is occuring?

@vladak
Copy link
Member

vladak commented Sep 29, 2017

The difference between link from directory listing and link from file display is that the former contains the a parameter, e.g. http://localhost:8080/source/xref/mercurial/Makefile?a=true, the latter too: http://localhost:8080/source/xref/mercurial/novel.txt?a=true&r=11%3A51bab7589f21.

Stepping through jsps files works for me in Netbeans 8.2 and Tomcat 8.

@vladak
Copy link
Member

vladak commented Sep 29, 2017

I think I see the problem in list.jsp, the r parameter confuses the logic there. The high level structure goes like this:

if (cfg.isDir()) {
  // directory listing
} else if (rev.length() != 0) { // this is true if r parameter is on
  // requesting a revision
  if (cfg.isLatestRevision(rev)) {
    // dump the file
  } else {
    // fetch historic revision and dump it
  }
} else {
  // requesting cross referenced file
  if (!cfg.annotate()) {
        // Get the latest revision and redirect so that the revision number
            // appears in the URL.   --- this is what adds the r? parameter
  }
  if (xrefFile != null) {
    // dump the file
  } else {
    // annotate
  }
}

So after the changes for #1736, the r parameter is always added when displaying a file and therefore the code in that case always goes to the second if branch. The else branch for annotate has to be refactored.

@vladak vladak self-assigned this Sep 29, 2017
vladak added a commit to vladak/OpenGrok that referenced this issue Sep 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants