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

fix: add label env to texMathText #2282

Closed
wants to merge 1 commit into from
Closed

Conversation

ld0891
Copy link

@ld0891 ld0891 commented Dec 15, 2021

Added \label{} environment to text-inside-math detection, along with a few tests.

@lervag
Copy link
Owner

lervag commented Dec 15, 2021

I'm not sure; \label is already matched with texCmdRef. The argument for \label is not normal text mode - it is simply a reference string for use in \ref commands. So I'm curious, what do you think is wrong with the current version?

@ld0891
Copy link
Author

ld0891 commented Dec 15, 2021

Ah I see, thanks for the prompt reply. The current logic in the function vimtex#syntax#in_mathzone(...) is first detecting texMathZone as well as texMathText, then excluding those matching texMathText. Now I understand that \label doesn't count as normal text mode, but should texCmdRef as well as texRefArg be excluded in that function as well? If so, this change is more suitable to be made in the function itself rather than the syntax part.

@lervag
Copy link
Owner

lervag commented Dec 16, 2021

Ah I see, thanks for the prompt reply. The current logic in the function vimtex#syntax#in_mathzone(...) is first detecting texMathZone as well as texMathText, then excluding those matching texMathText. Now I understand that \label doesn't count as normal text mode, but should texCmdRef as well as texRefArg be excluded in that function as well? If so, this change is more suitable to be made in the function itself rather than the syntax part.

Yes, perhaps; do you currently have any issues related to this particular thing? E.g. unexpected behaviour because the \label is matched as math environment?

@ld0891
Copy link
Author

ld0891 commented Dec 16, 2021

Yeah it's about completion. Right now there are different types of completion going on, one is math operators like \frac within math mode, another is environment like \begin{align} within text mode, yet another one is the reference strings you mentioned for \ref, which is in text mode as well. Plugins like UltiSnips use vimtex#syntax#in_mathzone(...) to decide which type of suggestions to make or expand. Right now if it's a \label within math mode, it still suggests/expands math operators, which is not the intended behavior. Please let me know what you think.

@lervag
Copy link
Owner

lervag commented Dec 16, 2021

Right now if it's a \label within math mode, it still suggests/expands math operators, which is not the intended behavior. Please let me know what you think.

Yes, I agree, except I don't see quite how this would be a real problem. I also use Ultisnips, but I never try to expand a snippet within \label{...}. I also have a couple of minor imaps (:help vimtex-imaps), but again, these are not relevant inside a label command and I don't have any issues with this.

I might still fix this, but it would be nice to see an actual real problem instead of just the theoretic fact that this is inconsistent/wrong. To expand on this: there are always going to be things that are not perfect, and I don't aim for that. Instead, I try to be pragmatic and address things that make a difference.

lervag added a commit that referenced this pull request Dec 16, 2021
@lervag
Copy link
Owner

lervag commented Dec 16, 2021

I believe this is addressed now.

@lervag lervag closed this Dec 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants