Add all root project env files to ImportantFiles

[haidubogdan]

Closes #9229

• add all root project env files to ImportantFiles
• additional coloring lexer fix for single quoted string with slash
• adjust EnvFileResolver to catch files starting with .env. in the filename

Coloring fix for single quoted string

There was a escape done on slash inside strings, which created a small coloring issue.

Before:

After:

File association

Before:
.env.local.demo was not under env mime type

After:

---

^Add meaningful description above

Click to collapse/expand PR instructions

By opening a pull request you confirm that, unless explicitly stated otherwise, the changes -

• are all your own work, and you have the right to contribute them.
• are contributed solely under the terms and conditions of the Apache License 2.0 (see section 5 of the license for more information).

Please make sure (eg. git log) that all commits have a valid name and email address for you in the Author field.

If you're a first time contributor, see the Contributing guidelines for more information.

If you're a committer, please label the PR before pressing "Create pull request" so that the right test jobs can run.

PR approval and merge checklist:

1. Was this PR correctly labeled, did the right tests run? When did they run?
2. Is this PR squashed?
3. Are author name / email address correct? Are co-authors correctly listed? Do the commit messages need updates?
4. Does the PR title and description still fit after the Nth iteration? Is the description sufficient to appear in the release notes?

If this PR targets the delivery branch: don't merge. (full wiki article)

[haidubogdan]

@mbien @matthiasblaesing

Just checking if the mime update is too extensive, and if it's ok for you to have all env files under ImportantFiles for web projects.

[mbien]

looks good to me but I have limited experience regarding php.

maybe @matthiasblaesing could take a quick look

[matthiasblaesing]

The matching is too broad. For example "test.env.js" or "test.env.ini" will also be matched and instead of being correctly handled as their correct mimetype env will occupy them.

I think it would be good to split the two different problems into different PRs. The change to the grammar makes sense to me. The broad mime type squatting does not.

[haidubogdan]

Thank you @matthiasblaesing .
I agree, the mime association can be a different PR.
I will have a second look to see if matching ".env." is enough for various frameworks.
But this might require double MIME registration to fix the ".test" mime type association.

[matthiasblaesing]

I will have a second look to see if matching ".env." is enough for various frameworks. But this might require double MIME registration to fix the ".test" mime type association.

Not sure that I get what you mean. test.env.js was just an illustation. The file could also be siteA.env.js, staging.env.js and so on. The filetype could also be .ts (I'm aware of the convention to call the file environment.ts in angular, but they could also be named staging.env.ts, .java or any other filetype.

The problem with .env files from my POV is, that they are an ad-hoc solution combined with a bad naming convention. From my POV it would much more sense to call the shown .env files dev.env dist.env and local.env. That way the OS won't hide the sample files and filetype matching is trivial.

What might be acceptable for the substring matcher would be to move the mimetype mapping far down and let it be considered last/late. That might need adjustment to other modules (the LSP integrations currently have a very high position, so this might need to be lowered).

[haidubogdan]

The existing MimeResolver service implemented in EnvFileResolver will not change the already associated mime type.
So, js, ini ... files will not be modified with the env mime type.
But I think the current implementation should be changed ... so basically any file which starts with ".env." can be associated with the env mime type. Technically it can be considered dot file.

My issue was with ".test" file extensions, I noticed that there are Symfony php Projects which use .env.test to specify a env file used for testing.
Overwriting this mime type can be done only with the custom mime_resolver.xml registration. But it's a nitpick.
And then it's a matter of excluding netbeans generated files :) . (.lexer, .error) .

I agree it is a bad naming convention, a bit confusing to find a pattern, but maybe we can find a balanced solution that can work for most of the IDE users.

[haidubogdan]

@matthiasblaesing could you help me with a feedback on the updates regarding the removal of mime-resolver.xml and correcting the mime association from files having the second element of env in their name ... to filenames starting with ".env." ?
Just to see if we keep the NB30 milestone for this PR.

[matthiasblaesing]

I think this is primarly important because of the lexer fix.

The mime-resolver is broad, but non-declarative mime resolvers are checked after declarative ones, so side effects should be minimal as most normal resolvers are consulted first.

The imporant files implementation will not pick up files added after the project was loaded. Before this PR ".env" would have been resolves, that is broken by this change.

[haidubogdan]

Ok, so I don't know if you are referring to this.
From what I see, live updates done on the root folder, like adding or editing the name of a file with env type mime, will not be seen in the ImportantFiles node.
When restarting the IDE it's all good.
Normally env files are quite static, and come with the project's framework, So, I never considered this.

[matthiasblaesing]

Yes that is what I mean. The important files mechanism (or the support class) in the currently used form supports only a static list of files. From a quick lock a different implementation is doable, but definitely out of scope for a late PR.

So I suggest to add ".env" even if is not present, that way we at least don't get a regression. I assume that only present files are actually shown. A comment should be added that documents that the implementation fails for files added inside the same session.

[haidubogdan]

Thank you for the observation.
I found that codeception had a similar need for the important files implementation :

netbeans/php/php.codeception/src/org/netbeans/modules/php/codeception/ImportantFilesImpl.java

Line 32 in 4c2749f

import org.netbeans.modules.php.spi.phpmodule.ImportantFilesImplementation;

So, I've moved the code handling the env mime files detection in the public Collection<ImportantFilesImplementation.FileInfo> getFiles() method.

[haidubogdan]

I found other issues which are a result of how extension detection is handled.
Renaming '.env.ext' can be a pain.
I'll research it and hope this can be fixed with file handling customization.

[matthiasblaesing]

This still does not work reliably. There is no trigger for updating the file list. This code:

https://github.com/haidubogdan/netbeans/blob/1177661a3d0eefbd4b58bba2d86ea1246fe42df8/ide/languages.env/src/org/netbeans/modules/languages/env/project/EnvFileImpl.java#L44

ensures, that file events touching files with the name .env will cause an update to the file list.

The code that handles that is here:

netbeans/ide/web.common/src/org/netbeans/modules/web/common/spi/ImportantFilesSupport.java

Lines 143 to 167 in 4c2749f

	private final class FilesListener extends FileChangeAdapter {
	@Override
	public void fileRenamed(FileRenameEvent fe) {
	check(fe.getFile().getNameExt());
	check(fe.getName() + "." + fe.getExt()); // NOI18N
	}
	@Override
	public void fileDeleted(FileEvent fe) {
	check(fe.getFile().getNameExt());
	}
	@Override
	public void fileDataCreated(FileEvent fe) {
	check(fe.getFile().getNameExt());
	}
	private void check(String filename) {
	if (fileNames.contains(filename)) {
	fireChange();
	}
	}
	}

and here:

netbeans/ide/web.common/src/org/netbeans/modules/web/common/spi/ImportantFilesSupport.java

Lines 67 to 73 in 4c2749f

	public static ImportantFilesSupport create(FileObject directory, String... fileNames) {
	Parameters.notNull("directory", directory); // NOI18N
	Parameters.notNull("fileNames", fileNames); // NOI18N
	ImportantFilesSupport support = new ImportantFilesSupport(directory, fileNames);
	directory.addFileChangeListener(WeakListeners.create(FileChangeListener.class, support.fileChangeListener, directory));
	return support;
	}

From my POV for this use case you'd need to create your own ImportantFilesSupport implementation that handles prefix and suffix matches.

The problem that can be observed with the current implementation is this:

1. Consider this start point:

2. Now I create .env.staging. This is not picked up:

3. Now I delete .env. This causes the list to be updated:

4. Now I delete .env.staging. The list is not updated:

I can double click on .env.staging, but nothing happens.

[haidubogdan]

Thank you,
As the ImportantFiles node is mostly a UI optimization experience I can give it more time to be developed.
Together with this I can see how renaming & copy flow can be improved.
So, I will try to merge just the fixes and skip this feature for NB30 milestone.