Migrate language modes to CSV

[FireChickenProductivity]

No description provided.

[FireChickenProductivity]

This is ready for review again.

[FireChickenProductivity]

Snippets apparently uses language_ids from language_modes.py, so it might be worth making sure that the snippets system still works.

The following code for dealing with snippets will not get reran on updating the CSV.

# Create a context for each defined language
for lang in language_ids:
    ctx = Context()
    ctx.matches = f"code.language: {lang}"
    context_map[lang] = ctx

I could address this by creating a callback registration mechanism inside language_modes.py.

[nriley]

Snippets apparently uses language_ids from language_modes.py, so it might be worth making sure that the snippets system still works.

The following code for dealing with snippets will not get reran on updating the CSV.

# Create a context for each defined language
for lang in language_ids:
    ctx = Context()
    ctx.matches = f"code.language: {lang}"
    context_map[lang] = ctx

I could address this by creating a callback registration mechanism inside language_modes.py.

We could declare an action that returns the list of languages?

[FireChickenProductivity]

We could declare an action that returns the list of languages?

Yes, but that would not cause the snippets code to update in response to the csv updating.

[nriley]

We could declare an action that returns the list of languages?

Yes, but that would not cause the snippets code to update in response to the csv updating.

Gotcha. Not sure if we have an example of this (I couldn't find one on a cursory search of community), but an action that triggers when the list is updated seems reasonable.

[FireChickenProductivity]

We could declare an action that returns the list of languages?

Yes, but that would not cause the snippets code to update in response to the csv updating.

Gotcha. Not sure if we have an example of this (I couldn't find one on a cursory search of community), but an action that triggers when the list is updated seems reasonable.

I plan on writing code to allow registering callbacks to handle language mode updates.

[FireChickenProductivity]

I theoretically have the code working, but I do not use the snippets system, so I will need to learn how it works before I can test it properly.

[FireChickenProductivity]

I did further testing of the snippet system, and it seems to be working with my changes. I consider the pull request ready for review, but someone more familiar with the snippet system than myself should make sure that my changes did not break anything.

[AndreasArvidsson]

Notes form their community session. We're not sure that introducing a event system especially for this use case is desirable.
I will have a look and see if I can get rid of the dependency on language ids from the snippets. Let's talk more about this on the next meetup.
If necessary another developer can finish this.

[FireChickenProductivity]

I am going to be busy for a while starting next week, so I am fine with another developer taking this PR.

[AndreasArvidsson]

Okay I looked into the snippet dependency on language ids and the result is not that good. Basically the only reason why I'm using the language ids from the language modes file and not the actual snippets themselves is because of performance. Dynamically creating contexts is problematic for Talon. Even to the extent that it have been suggested we should remove the custom snippet dir setting.

The current implementation creates all the contexts once before Talon is started/ready

community/core/snippets/snippets.py

Lines 32 to 36 in 9af1757

	# Create a context for each defined language
	for lang in language_ids:
	ctx = Context()
	ctx.matches = f"code.language: {lang}"
	context_map[lang] = ctx

Issue talking about the problem with updating snippet contexts
#1373

The implementation in this pull request removes the performance gain in creating the contexts before Talon starts. In that case we could just get the language ids from the snippet files themself. I can't really say how much of a problem it is to create the context dynamically, but I definitely feel that if we are going to create them dynamically we should not do it based upon the language mode file, but the snippet files themself.

[AndreasArvidsson]

If we decide to keep this implementation we definitely should check if the context already exists before creating a new one

[AndreasArvidsson]

My preferred solution would be to get the language identifiers from the snippet files themselves directly when the file is read so the context would be created before Talon is ready. The only scenario where we would dynamically create a context would be if you add a new snippet language which hopefully is not that common. The problem with this approach is that we have a setting for custom snippet dir. This setting is not available until Talon is ready. The implementation in this pull request might be the the best solution available at the moment since the language csv file can be read immediately at start.

[AndreasArvidsson]

How does this handle if the file doesn't exist?

[FireChickenProductivity]

Last time I tested the code, it created the file if it did not exist.

[knausj85]

Okay I looked into the snippet dependency on language ids and the result is not that good. Basically the only reason why I'm using the language ids from the language modes file and not the actual snippets themselves is because of performance. Dynamically creating contexts is problematic for Talon. Even to the extent that it have been suggested we should remove the custom snippet dir setting.

The current implementation creates all the contexts once before Talon is started/ready

community/core/snippets/snippets.py

Lines 32 to 36 in 9af1757

	# Create a context for each defined language
	for lang in language_ids:
	ctx = Context()
	ctx.matches = f"code.language: {lang}"
	context_map[lang] = ctx

Issue talking about the problem with updating snippet contexts #1373

The implementation in this pull request removes the performance gain in creating the contexts before Talon starts. In that case we could just get the language ids from the snippet files themself. I can't really say how much of a problem it is to create the context dynamically, but I definitely feel that if we are going to create them dynamically we should not do it based upon the language mode file, but the snippet files themself.

To me, it sounds like we might be better off keeping the language ids in python. Maybe we should instead clean up the extension definition stuff e.g. using a data class to define the list of extensions/associated spoken forms to make it more easily understandable/modifiable, and live with the need to branch this particular file for performance reasons. Plus, the alternative solutions to this PR require a talon restart if we modify the file, which is downer.

[AndreasArvidsson]

In the community session today we talked further on this topic. We think we should probably have a new python file code_languages.py where we have the language definitions. This python file can then be used by the language modes and the snippets.

We want to have these language definitions typed and something like this could probably work:

string_or_list = str | list[str]
lang = tuple[str, string_or_list] | tuple[str, string_or_list, string_or_list]

languages: list[lang] = [
    ("python", ["python"]),
    ("java", "java"),
    ("javascript", ["javascript"], ["something something"]),
]

[nriley]

Migrated to an issue (#1585).