-
Notifications
You must be signed in to change notification settings - Fork 65
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
Adding autoformatting #296
Comments
First of all the Display trait is a dead end for formatting. To do proper formatting you need to have a context such as the current indentation level and probably also settings.
The current architecture has been to not include comments in the AST but rather have each AST be aware of the span of Tokens it consists of. The comments can be extracted from the token span and be kept in this way. Each Token is associated with either leading or trailing comments. The comments are stored in the tokens and are not tokens themselves. This separation between AST and Tokens is to keep the different layers of the architecture simpler and focused om their role. To have all information in one big structure would be very complex and also all information is not availablein a single pass. VHDL-LS has three clear layers:
Layers willhave references to each other, for example AST will reference a span of tokens. AST will also reference named entities which are either decarations or uses of for example signals, entities etc.
Rust format is a good guiding star to lead the way, its a very nice tool. Of course you got to start somewhere and it will not be as good as rustfmt in the first iterations. |
Just want to add that I am very interested in pushing this forward. Should anyone need any help (and I know that implementing such capabilities can mean a lot of work), feel free to contact me |
Thanks for the feedback @kraigher and for the offer @Schottkyc137. I'm interested in working on this, I'll start doing some initial experiments and see if I get stuck anywhere. |
One thing that currently hinders a straightforward implementation of such features is that tokens are not always part of the AST. Instead, some AST elements are enriched with the source code information. Do you think that, in the long run, this source code information should entirely be replaced with the token information? |
Yes I think all AST nodes should have information on which token span they come from in the form of 32-bit indexes into the tokens of the file. In most cases this would mean you could remove the WithPos etc from AST nodes. Sure it will be a lot of grinding to achive this but I do not see a better way forward. Whay would be the alternative? Somehow adding comments directly to the AST would be even more work, less elegant and would not preserve information about other aspects that a formatter would care about such as extra paranthesis and casing. |
Maybe this project is relevant to the conversation. |
There has already been some discussion over there from @kraigher jeremiah-c-leary/vhdl-style-guide#920 |
Morning everyone,
I am the creator of this project and have spent over 6 years on it's development. I believe I am at the point where I need to re-evaluate my implementation and was wondering if using rust_hdl would be an option to build upon. Honestly not sure where to start, but maybe this question will do: Is there an API documented somewhere? I tried, briefly, to look for something but I did not see a link to one. Regards, --Jeremy |
Unfortunately there is no stable API yet. Attempts have been made, but not much has happened since quite some time. One of the main reasons (at least according to what I think) is that there are still a few fundamental design decisions that have to be made before one can publish an API. For example, for the formatter, a lot of changes concerning the AST were necessary, mostly to incorporate source locations into the nodes. |
Thanks @Schottkyc137 , I looked over this thread and this pull request. It looks like some form of output formatting has been implemented. How do you see this collaboration working out? My knowledge of rust is limited at best, so code contributions would be out of the question. I do have a lot of experience on the user side of formatting VHDL code, what users are looking for and their use cases. --Jeremy |
So I'm not quite sure what the best model is here. But what is lacking from the formatter as designed in #321 is some way of enabling user input. So basically, what I am looking for is a "configuration blueprint". What is it that users actually want to configure? On of the problem that I see with this "proposal" is that there is a lot of commonality in this project and yours. Essentially, after all is done, |
Morning @Schottkyc137 ,
Users want to configure everything. VHDL does not have a standard formatting guide similar to Python's PEP8. All formatting decisions are left to the user, and like anything left to the users there will be variations. If you have not reviewed the documentation for VSG, here are some examples: We went through a whole discussion on tabs vs spaces, treating prefixes and suffixes of names differently than the base name. Some requests have been quite surprising to me as I would never have thought to format code in requested manner. I wrote some documentation on how VSG rules should be created. I wish I had taken the time to create that document before I started coding. I believe there are 900 rules for formatting code, but that is a small fraction that could be created.
I was curious about the roadmap for
Is adding formatting to The key to enabling other use cases is a good API. If the formatting feature you are adding is a separate tool, then it would force the development of the API so others can use it effectively. At the moment, the formatting is within As for cannibalizing VSG, I have mixed feelings. It has been an amazing journey for me and has pushed my coding skills forward. I have had the opportunity to talk with people from around the world and at places like CERN. At the same time it has taken a large amount of my time and I can not focus on other areas of interest: Cannibalizing would only occur if formatting was added to
I am more than willing to share my experiences in creating VSG and any lessons learned. Would you want to start with the #321 pull request. I do have questions about the data structure and opinions on how the formatting should be implemented. Regards, --Jeremy |
Good evening @jeremiah-c-leary
I read the document on how to create rules. This is really interesting and might give a good lead. I have also read some of the documentation. It's truly amazing how many different styles there are :D
In my opinion, it's not. There are two main reasons:
In general, the roadmap resp. goals of this project are – broadly speaking
The use-cases you outlined (e.g., block diagrams, code statistics, compile order, e.t.c.) fall under the second category. I find both of these use-cases interesting, but will mostly work on the former (out of personal interest). That being said, I'm also more than happy to help and put work into creating a good API if there are concrete use-cases and helpful user input.
I will definitely add the formatting capabilities (and I have merged the PR). It's simply too much work that has gone into the tool. In what capacity this is going to be integrated into the whole eco-system is another question though.
Even though it's already merged I would really love to hear your opinions and I'm happy to answer any questions of yours. Best, |
Hello Lukas, Jeremy, and others, I tried the formatting from the latest master branch. Worked well on the 30k lines of code across 160 files that I tested it on. Really cool to see, great job! Some thoughts:
I would be very happy to assist in discussions, code reviews, trying new things out, etc. Other than that, I don't really have the Rust knowledge nor the computer science or compiler tech background to understand these things. And I can't allocate time for even more open source work. So unfortunately, I won't contribute any code, but I hope I can be of assistance anyway. |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Intro
I'm investigating how hard it would be to add automating as a feature, as part of #156. This project already has a parser, now we "only" need to add the other direction 🙂.
Inspiration can be taken from rustfmt, notably
Design.md
and the more up-to-dateContributing.md
.Current state
I noticed the file
display.rs
has implementations for some, but not all, AST nodes. As far as I can tell statements and anything that would require them (up to and includingAnyDesignUnit::Secondary
) does not yet have aDisplay
implementation.Is the intention that this gets extended to cover the entire AST?
Some limitations of the current design:
Future
Some decisions need to be made:
I'll do some more research and maybe start on a prototype, I just wanted to start the discussion as early as possible.
The text was updated successfully, but these errors were encountered: