eclipse / xtext

Eclipse Xtext™ is a language development framework
http://www.eclipse.org/Xtext
Eclipse Public License 2.0
770 stars 321 forks source link

Validation Jobs on Readonly Editors #2378

Open cdietrich opened 8 years ago

cdietrich commented 8 years ago

ValidatingEditorCallback does not schedule validation jobs on read-only editors but org.eclipse.xtext.ui.editor.model.XtextDocumentProvider.registerAnnotationInfoProcessor(ElementInfo) does. is this a indended behaviour? or a bug?

cdietrich commented 7 years ago

any comments?

svenefftinge commented 7 years ago

It is intended to not run validation on readonly editors, since the user couldn't do anything about the problems anyway. It is inline with the behavior in Eclipse elsewhere (and also VSCode)

svenefftinge commented 7 years ago

Sorry, I should read more carefully :)

cdietrich commented 7 years ago

i had a look and i do not really see a clean solution for that. what might work is something like

public interface IValidationJobCallback {

    public boolean isReadOnly();

}

in XtextDocumentProvider

protected void registerAnnotationInfoProcessor(ElementInfo info) {
        XtextDocument doc = (XtextDocument) info.fDocument;
        if(info.fModel != null) {
            AnnotationIssueProcessor annotationIssueProcessor = new AnnotationIssueProcessor(doc, info.fModel,
                issueResolutionProvider);
            ValidationJob job = new ValidationJob(resourceValidator, doc, annotationIssueProcessor, CheckMode.FAST_ONLY);
            job.setValidationJobCallback(new IValidationJobCallback() {

                @Override
                public boolean isReadOnly() {
                    return info.fElement != null && XtextDocumentProvider.this.isReadOnly(info.fElement);

                }
            });
            doc.setValidationJob(job);
        }
    }
    @Override
    protected void disposeElementInfo(Object element, ElementInfo info) {
        if (info.fDocument instanceof XtextDocument) {
            XtextDocument document = (XtextDocument) info.fDocument;
            ValidationJob job = (ValidationJob) document.getValidationJob();
            if (job != null) {
                job.cancel();
                job.setValidationJobCallback(null);
            }
            document.disposeInput();
        }
        super.disposeElementInfo(element, info);
    }

in ValidationJob

    @Override
    protected IStatus run(final IProgressMonitor monitor) {
        if (monitor.isCanceled())
            return Status.CANCEL_STATUS;
        if (callback !=null && callback.isReadOnly()) 
            return Status.OK_STATUS;
                 .....
    private IValidationJobCallback callback;
        public void setValidationJobCallback(IValidationJobCallback callback) {
        this.callback = callback;
    }

@svenefftinge what do you think? do you have a tipp for me?

szarnekow commented 7 years ago

We should keep in mind that there still are pessimistic VCS out there. They will mark files as readonly in the editor. Validation should not be disabled completely. Also builders will happily build and compile readonly files and report errors / warnings. These should be exposed when opening the editor, too. But the question is: which problem do you want to solve? Assuming you have a readonly file open in an editor and you change a referenced file. When this change breaks the file in the readonly editor, it should be highlighted as a an error. Without a validation job, this would not be detected before saving the edited file, would it?

cdietrich commented 7 years ago

=> it is intended not do validate in ValidatingEditorCallback but to validate in DocumentProvider?

cdietrich commented 7 years ago

here is the trace that schedules the job

XtextDocument$XtextDocumentLocker.modify(IUnitOfWork<T,XtextResource>) line: 476    
XtextDocument$XtextDocumentLocker.process(IUnitOfWork<T,XtextResource>) line: 337   
XtextReconciler$DocumentListener.performNecessaryUpdates(IXtextDocumentContentObserver$Processor) line: 130 
XtextDocument.updateContentBeforeRead() line: 249   
XtextDocument$XtextDocumentLocker.internalReadOnly(IUnitOfWork<T,XtextResource>, boolean) line: 518 
XtextDocument$XtextDocumentLocker.readOnly(IUnitOfWork<T,XtextResource>) line: 492  
XtextDocument.readOnly(IUnitOfWork<T,XtextResource>) line: 133  
HighlightingReconciler$2.run(IProgressMonitor) line: 336    
Worker.run() line: 56   
cdietrich commented 7 years ago

or asked the other way round: if the reconciler validates anyway. why do we validate in the callback on startup

cdietrich commented 7 years ago

our usecase is the following: we have files (that might not be competely valid) that the user can open/watch but not edit. if the validation runs it might produce errors that the user shall not see (since he/she cannot fix the problems anyway)