thSoft / elysium

LilyPond IDE for Eclipse
http://elysium.thsoft.hu
14 stars 3 forks source link

recompile action enabled only if executable #160

Closed nittka closed 6 years ago

nittka commented 6 years ago

This PR disables the recompile action in the menus, if a recompile is not possible, e.g. for files opened in read-only mode (because there is no IFile that could be recompiled) or because the selection in the score library is not a lilypond file.

A further improvement on the RecompileSelectedHandler would be allowing recompile on compilation results (activate and execute recompile on a score.pdf). If you think this to be a sensible improvement, I would add a corresponding commit to the PR.

thSoft commented 6 years ago

Yes, I think recompiling the source file corresponding to the result file would make sense (like the Recompile Viewed option), I'd be thankful if you made this improvement.

nittka commented 6 years ago

Done.

nittka commented 6 years ago

Depending on #166 the logic should be extended (regarding .ily files as non-recompilable). I'd like to add a commit to this PR, once we have decided on the best option for dealing with ily files (separating syntax update and compilation should be a separate step I guess).

nittka commented 6 years ago

This PR modifies the recompile action activation and modifies the compilation process slightly. Recompile "edited" and "selected" are possible only for ly files and compilation results (score in particular). The outdated marker creation was moved into the regular builder - a workspace resource change may have been detected even if the file content was really not changed at all (syntax update which does nothing).

Our builder uses the index information (recompile hash in particular) in order to determine, whether a LilyPond compile is necessary. Hence, it knows if a file is dirty and may correctly mark it.

Invoking LilyPond compilation is restricted to ly files (so is dirty marker creation and removal).

Finally, the LilyPondBuilder will trigger only for incremental builds (if a file is saved after modification). Clean builds will only update the Xtext index etc. and not rebuild all scores in the workspace.

Once you have reviewed this PR, I would rebase the index based inclusion calculation, wrapping up #147 - either adding it to this PR if you want that or creating another one.

nittka commented 6 years ago

I added the commit that modifies the determining of including files from reparsing to using index information.

Please consider my response to your review comment. It is indeed not the case that the outdated marker creation is deactivated by deactivating the compiler.

thSoft commented 6 years ago

Sorry, I misphrased my request. My problem is that the outdated marker is deactivated when Build Automatically is disabled. Do you see a way to circumvent this? If it remains disabled accidentally, the user won't know that the results are outdated.

nittka commented 6 years ago

I see. Deactivating automatic build will indeed remove the outdated marker creation. However, it will also deactivate linking (the entire Xtext linking infrastructure) and index update etc., basically making much of Elysium completeley useless. So the loss would not be that much bigger.

In my opinion, the advantages of the change outweigh the disadvantages (which exist only, if you use Elysium just as a plain text editor)...

In fact, I don't see a reason why automatic build should be disabled when working with Elysium. Long ago the automatic compiler run was a good reason, but now you can disable that during build. It's like turning off electricity and then complaining that it's dark.

If the build is disabled accidentally, I think it is still better if the markers were not added - as additional indicator to the user that something is wrong. If accidental deactivation is your concern, we should add a check that automatic build is active (similar to the check that the Xtext nature is active for projects with Xtext models) - in a new ticket.

thSoft commented 6 years ago

Okay, I'm convinced, let's go this way!