Skip to content
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

recompile only outdated when container is selected #184

Merged
merged 2 commits into from
May 30, 2018

Conversation

nittka
Copy link
Collaborator

@nittka nittka commented May 9, 2018

As recompilation is not possible on an ily file anymore, it may be less easy to invoke recompilation on all including files (which may be spread over different folders), especially if compile on save is deactivated.
This pull request introduces a preference value controlling whether all files or only those marked as outdated will be processed, when recompilation is invoked on a folder or a project.

(My hope was that this makes #127 obsolete, as the Elysium's compiler preference page is completely different from the standard Xtext preference page and hence allowing project specific settings may not be easy. However, the use case there - compile on save in one project but not another - remains)

@thSoft
Copy link
Owner

thSoft commented May 10, 2018

Yes, this behavior needs to be disambiguated. I think the user should make it explicit whether (s)he wants to recompile every file or just the outdated ones, so I suggest that a container should have two separate commands.

@nittka
Copy link
Collaborator Author

nittka commented May 10, 2018

So, rather than the preference, a "Recompile outdated" command should be added (and the other be renamed to "Recompile all")? Recompile viewed and recompile edited are not affected.
Note, that the selection may involve both multiple single files and multiple containers.

@thSoft
Copy link
Owner

thSoft commented May 10, 2018

Yes, this would be great.

@nittka nittka force-pushed the recompileOutdated branch from 6f9504d to 5ad0616 Compare May 11, 2018 11:08
@nittka
Copy link
Collaborator Author

nittka commented May 11, 2018

I changed the commit. There are now 2 recompile selected actions - all and outdated only.

@nittka nittka added this to the 0.6.0 milestone May 11, 2018

@Override
public Object execute(ExecutionEvent event) throws ExecutionException {
compileOutdatedOnly = event.getCommand().getId().endsWith("outdatedOnly");
Copy link
Owner

Choose a reason for hiding this comment

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

Please add a commandParameter to the command in the plugin.xml instead of deciding based on the command's name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried that. But I failed. Distinguishing the cases using command ids seemed a valid alternative. But I will check again, if I somehow can add a dummy parameter to the event.

@nittka
Copy link
Collaborator Author

nittka commented May 21, 2018

It seems, a command parameter would make sense only if there was only one recompile command and the user is asked every time whether all or only outdated files are to be compiled. Is that what you intended?

Otherwise two command IDs are necessary anyway and distinguishing the two cases by evaluating the ID seems a valid approach.

@thSoft
Copy link
Owner

thSoft commented May 25, 2018

There can be one command definition (i.e. a command element under the org.eclipse.ui.commands extension point) which has a commandParameter, and two menu commands (i.e. command elements under the org.eclipse.ui.menus extension point) which can specify arguments (i.e. parameter elements) for the called commands.

@nittka nittka force-pushed the recompileOutdated branch from 5ad0616 to bef584d Compare May 26, 2018 11:52
@nittka
Copy link
Collaborator Author

nittka commented May 26, 2018

Thanks for the hint. I missed the possibility of providing the command parameter in the menu entry.
I added the command parameter, set the value in the compile-outdated menu entry, rebased on master and modified the original logic slightly. Rather than restricting the "outdated" logic to containers, now from all selected elements only outdated files are compiled.
I.e. the menu entry Recompile All compiles all ly files (derived from selected files and containers), and Recompile Outdated compiles only those which are marked as outdated.

If we wanted enablement of the entry to work perfectly (Recompile outdated is enabled only if there are outdated files) we would have to use a more elaborate logic, e.g. separate commands and handlers, because the isEnabled-method lacks the information which of the two menu entries is asking.

@thSoft
Copy link
Owner

thSoft commented May 30, 2018

Thank you, this is sufficiently sophisticated for now.

@thSoft thSoft merged commit 9c78d0d into thSoft:master May 30, 2018
@nittka nittka deleted the recompileOutdated branch May 30, 2018 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants