eclipse / Xpect

This repository has been rewritten to move to the Eclipse Foundation. Find the old history here: https://github.com/TypeFox/Xpect
http://www.xpect-tests.org/
Eclipse Public License 2.0
30 stars 28 forks source link

negated editor override; extensions will be included instead of excluded. #358

Closed mehmet-karaman closed 1 day ago

mehmet-karaman commented 3 weeks ago

@iloveeclipse please could you check this PR. Is it correct now?

cdietrich commented 3 weeks ago

am not sure if we want to break existing xpect users. this will break them. correct? or will the list be empty, and thus it will be checked?

what about adding a flag to the extension point. if i remember correct you are using it.

iloveeclipse commented 2 weeks ago

am not sure if we want to break existing xpect users. this will break them. correct?

No, if (by default) nothing is specified in preferences, nothing will change.

or will the list be empty, and thus it will be checked?

Exact

what about adding a flag to the extension point. if i remember correct you are using it.

Which flag?

cdietrich commented 2 weeks ago

can you share the xpect extension point you are using. (my gedächnis aint so good to remember how it exactly looks like)

mehmet-karaman commented 2 weeks ago

Maybe I am misunderstanding something :D We didn't implemented or adjusted any extension point.

Maybe its better to explain the motivation for the changes: Lets assume that there are Xpect users and non-xpect users sharing the same projects. Lets also assume that there are files containg the text "XPECT" in any combination in text or comment etc. When a non-xpect user opens these Files, Xpect is forcing the opening of the files in the Xpect editor instead of their default text editor. The xpect editor is an Xtext Editor which forces also the user to add Xtext Nature on that project, but for remembering the user isn't an xpect user.. He was just forced to do so..

Our proposed solution: We give the ability to add file extensions in the xpect preference page (semicolon separated string which is used as included file extensions) to use them as inclusion. If the list is empty (by default it is), it wont affect anyone. If the user adds a File extension, only these file extensions content will be checked for the text "XPECT" and will be overriden same as before. All others file contents won't be checked and their editor won't be overriden.

cdietrich commented 2 weeks ago

as i said: i dont know who is using xpect and i wonder if we can make this really opt out can you please grep your plugin.xmls for xpect.

for (IExtensionInfo ext : registry.getExtensions("org.eclipse.xpect.fileExtensions"))

i assume you use that one

iloveeclipse commented 2 weeks ago

i dont know who is using xpect and i wonder if we can make this really opt out

The patch shouldn't affect anyone, it only works if a new preference is set. No one has it set, so no one will be affected.

mehmet-karaman commented 2 weeks ago

I've read out all the implemented extensions in the target: `

`

But in runtime mode i am not able to know, which extension is going to be implemented by the user in the target platform. And thats not our only problem, for example a *.sh (shellscript which contains XPECT as text in any combination) is not a dsl file:

image

isXtFile(..) method will return in any case false, but Eclipse will open this file also with the Xpect editor (and then the user will be asked for Xtext Nature etc..)

cdietrich commented 2 weeks ago

yes this is why i am asking about the extension point. if one registers sh there then its a known xpect file, if not, then not

i assume your have something like

<fileExtension fileExtension="yourdsl" in your codebase and the question is if we can add a

useEditorAssocOverride="true/false" there with default true

mehmet-karaman commented 2 weeks ago

The user implements his dsl tests in the runtime itself.. In the runtime he has also implemented his DSL.. So this extension point won't help us, because when he implements this extension, xpect would notice it only in the target platform. But in most of the cases the target platform doesn't contain the xpect editor.. its only necessary in the runtime for development and test purposes.

cdietrich commented 2 weeks ago

i guess i am too busy to follow. xpect files are usually developed in a system where the dsl is present. maybe you are an exception for that

but if the file is not present then no extension point will be present => answer to known xpect language will be false

cdietrich commented 2 weeks ago

@kbirken @grafandreas are you still using xpect. any opinions?

iloveeclipse commented 2 weeks ago

Just to make sure we are all on same page: this PR is a negation of conditions introduced in https://github.com/eclipse/Xpect/pull/354 that was merged last week, without much trouble.

Instead of providing which files we shouldn't open in Xpect editor we provide files that only should be opened in Xpect editor if the preference is set.

In both cases nothing changes for anyone without new preference, and no one has the new preference yet because https://github.com/eclipse/Xpect/pull/354 was merged last week.

cdietrich commented 2 weeks ago

yes, understood, but if you add more languages you need to remember you have the preference set and opt in for your languages

iloveeclipse commented 2 weeks ago

@mehmet-karaman : can we have a checkbox "enable xpect editor only for registered file extensions" and implementation would scan through all registered languages in the target platform (once), creates the list and checks the list if the checkbox is enabled?

Damn, I'm now confused again, sorry, too much of different tickets.

Just ignore my comment, I will delete it. Of course in the IDE runtime, where the editor is opened, there are no language extensions configured that only known by the target platform.

iloveeclipse commented 2 weeks ago

Ping?

iloveeclipse commented 1 week ago

May be we could proceed here?

cdietrich commented 1 week ago

I am on vacation and have no code access I am still confused about which platform we are talking about

I thought about the one where the dsls are installed and registered to xpect using xpects extension point

iloveeclipse commented 1 week ago

I thought about the one where the dsls are installed and registered to xpect using xpects extension point

No, we are talking about IDE that is used to develop DSL's, so they aren't installed.

cdietrich commented 1 week ago

Hmmmm My proposal was to take the installed / Registered extensions as a based for an opt out and don’t support others at all (maybe we can have an add. Opt in)

unfortusltely no feedback from others users so nobody seems to care .

maybe people also don’t install xpect.ui into their developer tooling but their runtime tooling only

mehmet-karaman commented 1 week ago

@cdietrich I've adjusted the solution according to your suggestion. Now it looks more clear and it should be easier to understand what's going on there..

mehmet-karaman commented 2 days ago

@cdietrich can you look into the *.target what was changed with my last commit (the build wasn't succesffull due to the 4.33-I repository wasn't reachable, I've had to adjust this to pass the build successfully). Are there any other open questions or are we done with this PR?

cdietrich commented 2 days ago

Can I currently have no commit rights due bureaucracy I cannot do anything

cdietrich commented 1 day ago

can you please rebase

mehmet-karaman commented 1 day ago

Rebase done, build is green again :)

cdietrich commented 1 day ago

thx

iloveeclipse commented 1 day ago

@cdietrich : thanks and congratulations for having your new (old) commit rights again :-)

Speaking about commit rights: do you think @mehmet-karaman could become committer on xpect repo?

There seem to be not many people active on the project, we want Mehmet to help with most obvious tasks like fixing builds etc.

cdietrich commented 1 day ago

hi is mehmets eclipse mail the advantest one?

cdietrich commented 1 day ago

https://projects.eclipse.org/projects/modeling.xpect/elections/election-mehmet-emin-karaman-eclipse-xpecttm

mehmet-karaman commented 1 day ago

Hi Christian, Thank you for helping me to upggrade to a xpect commiter user.. :)