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

Support zero-indexed line numbers in TextArea #4471

Merged
merged 5 commits into from
Jul 11, 2024

Conversation

royatt
Copy link
Contributor

@royatt royatt commented May 2, 2024

Possible alternatives:

  • Add this as a keyword argument - IMO this use-case will not be common enough to deserve it's own typed keyword argument being there until the end of time
  • Make the show_line_numbers kw accept an int (with -1 signifying False) - won't add another kw but will terribly hurt readability
  • Split up the source code in such a way it won't require overriding the entire render_line method to do this in one's own project - not worth it, will probably end up being very invasive, while this approach is fairly elegant

Something I still consider adding:

typing this class variable as : Literal[0, 1] to increase clarity of what it's use case really is

@royatt royatt force-pushed the text-area-lines-from-zero branch from cebfe20 to 4b13b49 Compare May 2, 2024 22:20
@darrenburns
Copy link
Member

I don't really understand the benefit of this - why would you want to have the line numbers be zero indexed?

@davep
Copy link
Contributor

davep commented May 3, 2024

Chiming in with a perspective: I can't personally see any benefit in zero-based line numbers, as a specific feature, but I can see benefits to being able to set the origin number of the line numbers. For example, I might want to create a breakout editor where I'm working on a specific few lines of code, and I want to maintain the line numbers; another reason might be I want to do some sort of narrowing effect.

So perhaps if such a feature were added in some way, it would make sense that it's a way of setting the number of the "first visible line"?

@darrenburns
Copy link
Member

@davep Yeah, I can see that being useful. Rich Syntax offers something similar IIRC.

@royatt
Copy link
Contributor Author

royatt commented May 4, 2024

I don't really understand the benefit of this - why would you want to have the line numbers be zero indexed?

The use case I was going for is I have a sort of a query language embedded into a TextArea that you can submit and then it shows you results (kinda like that cool SQL TUI project named harlequin).

I also have this cool feature where when the language grammar is parsed; if you have a syntax error you get a toast with the error the parser created.

The problem is, the parser shows line number based errors which are zero-index based :(

I did some googling (for this problem in text editors/ programming in general) and found some more use cases, so I'm not totally alone on this it seems (albeit, yes - This might not be a very intuitive thing to add)

@royatt
Copy link
Contributor Author

royatt commented May 4, 2024

Chiming in with a perspective: I can't personally see any benefit in zero-based line numbers, as a specific feature, but I can see benefits to being able to set the origin number of the line numbers. For example, I might want to create a breakout editor where I'm working on a specific few lines of code, and I want to maintain the line numbers; another reason might be I want to do some sort of narrowing effect.

So perhaps if such a feature were added in some way, it would make sense that it's a way of setting the number of the "first visible line"?

Hmm, this can be fairly easily transformed into an instance attribute with no further cost if you think it's better

@willmcgugan
Copy link
Collaborator

I think this is probably best done as a reactive on the widget itself. Possibly also exposed in the constructor.

Suggest we call it line_number_start

@merriam
Copy link
Contributor

merriam commented May 27, 2024

The use case I could possibly imagine:

My program is popping up a text area to modify one little section of a large annoyingly formatted data file. For example, "change some parameters for the machine Gandalf, but don't let anything else change". The relevant section would be loaded into the buffer, the buffer would be edited in a TextArea, and, after some validation, the annoying file is updated. In that case, setting the starting number of a text area would be helpful.

@willmcgugan
Copy link
Collaborator

@royatt I think this is a good idea, but its probably best done as an argument on the constructor. Would you like to make that change?

@royatt
Copy link
Contributor Author

royatt commented Jun 17, 2024

@royatt I think this is a good idea, but its probably best done as an argument on the constructor. Would you like to make that change?

Hey, it would take me a couple of days to get to it but yes I'd like to do it

@royatt royatt force-pushed the text-area-lines-from-zero branch from 4b13b49 to 701308c Compare June 22, 2024 19:03
@royatt
Copy link
Contributor Author

royatt commented Jun 22, 2024

@royatt I think this is a good idea, but its probably best done as an argument on the constructor. Would you like to make that change?

Just pushed an amended commit where line_number_start is a reactive keyword argument

@darrenburns
Copy link
Member

I added a snapshot test and updated the docs.

Works great:

Screen.Recording.2024-07-11.at.11.27.52.mov

Copy link
Member

@darrenburns darrenburns left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@darrenburns darrenburns merged commit b61028d into Textualize:main Jul 11, 2024
20 checks passed
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.

5 participants