universal-ctags / citre

A superior code reading & auto-completion tool with pluggable backends.
GNU General Public License v3.0
337 stars 26 forks source link

Rearrange code and add tests #8

Closed AmaiKinono closed 4 years ago

AmaiKinono commented 4 years ago

First, I don't plan to work on this soon. But we sure need tests, so discussing it now is not bad ;)

In https://github.com/AmaiKinono/citre/pull/5, @masatake suggested:

I think using an emacs lisp file like citre.el as a target input is not a good idea. Because Emacs provides so much good things for elisp. As the result, we can miss a hidden bug in citre. I think we should choose another language as the target.

https://github.com/universal-ctags/codebase/tree/master/lcopy.d Here, I listed repositories I used for testing u-ctags. All of them are large. lsof is not so large. However, it is written in very old style C.

u-ctags itself is not bad target. However, we have to make Units and Tmain excluded when making a tag.


My thought about this:

I think using an emacs lisp file like citre.el as a target input is not a good idea. Because Emacs provides so much good things for elisp. As the result, we can miss a hidden bug in citre.

I'm actually not sure if you are talking about automated test or test "by hand". If we test by hand, sometimes we may use built-in functionalities rather then those offerd by Citre due to habit, and that causes the problem you described. But this won't happen in automated tests.

And, here's the problem: How are automated tests possible for code map? Well, I thought about it. My conclusion is it may actually be easier than testing functionalities in citre.el. Because if you look at those code map commands, most of them don't ask for user inputs, just run it and it will do something. So we could make Emacs open a file, jump to a position, call citre-see-symbol-in-code-map, the run various commands and see if the contents of current line/buffer is as expected. This shouldn't be hard to automate.

These are just some thoughts. For code map, we may need automated tests or not.

think we should choose another language as the target...

If using automated tests, I would like to use some "mini" projects that's suitable for directly including in the Citre repo, so future contributers don't need to clone another repo just to run the tests. In fact, I'm almost sure we will need to do many language specific things in citre.el, so we will need mini projects of different languages for testing.

masatake commented 4 years ago

I'm thinking about testing the LOWER layer of citre in automated way.

I think in concept level, we can split citre in 3 layers.

  1. the lower one is related to xref. From my short study of etags.el, I guess we can remove the concept "project" from the layer. This layer may have routines for running readtags and reading its output. This area should be tested extensively in automated way. I"m thinking about submitting as a standalone .el to emacs-devel list IF YOU ARE O.K. (However, for submitting, I must release a version of u-ctags.).

  2. the middle one is a layer including citre-peek that I like. In this layer is based on 1. So if 1. is broken, this layer doesn't work.

  3. the higher layers including map. It depends on 1. and maybe 2.

Of course, I would like to use 3. However, 1. is important. I would like to work on improving 1.

If using automated tests, I would like to use some "mini" projects that's suitable for directly including in the Citre repo, so future contributers don't need to clone another repo just to run the tests. In fact, I'm almost sure we will need to do many language specific things in citre.el, so we will need mini projects of different languages for testing.

The mini project is needed even for testing 1.

AmaiKinono commented 4 years ago

I think in concept level, we can split citre in 3 layers...

That's exactly what I have in mind :D. Here's my 3 layers:

  1. Main APIs and anything lower than that.

  2. Auto-completion and jumping (including xref and citre-peek) functionalities, including some helpers and commands.

  3. Citre Map. It depends on 1, and helpers in 2.

I think we could further divide 2 into two layers, one is some generic helper functions for auto-completion and jumping, the other deals with the UI, which can also include Citre Map.

I'm thinking about submitting as a standalone .el to emacs-devel list IF YOU ARE O.K.

I am ok ;) And, just a guess, your patch may have more chance to be merged then my.

...including citre-peek that I like.

Out of curiosity, can I hear why you like it? It's designed as a tool for code writers, and from what I know you are mainly a code reader.

masatake commented 4 years ago

Out of curiosity, can I hear why you like it? It's designed as a tool for code writers, and from what I know you are mainly a code reader.

After trying ctire-peek, I found there are two classes of UI for code reading. A. google map/goole earth class B. HUD (https://en.wikipedia.org/wiki/Head-up_display) class

I have not recognized the class B. till trying citre-peek. The most of .el I have ever written can be classified to A.

eldoc can be classified to B. However, it is ephemeral. citre-peek is classfied to B and its lifetime(?) is controllable. If I want to keep it long, it can be. If I don't want, I can hide it.

When I'm trying citre-peek, a word "escort" hits me. citre's overlay escorts the person in long journey in too complicated source code.

When doing citre-jump in citre-peek, the overlay is not displayed at the place where I jumped to. It will be nice if the overlay is shown continuously. The person must be escorted always. When jumped at, I would like to see the information about where I jumped from (the information about parent context).

For me, the overlay of citre-peek is massively extended version of "point".

If what I wrote doesn't contradict your design of citre-peek, I recommend you to give a peculiar name. "escort" is my idea. However, as far as reading dictionary, it has multiple meanings. So I'm not sure it is acceptable or not. Another one is "agent". However, the word becomes not so impressive these days.

Anyway, I would like to focus on "Main APIs and anything lower than that".

citre-peek can list multiple items having the same name. It will be nice if we can sort them. I'm working for readtags side for the purpose: https://github.com/universal-ctags/ctags/pull/2519#issuecomment-617486118

AmaiKinono commented 4 years ago

Ah, I see ;) this is very informative.

eldoc can be classified to B. However, it is ephemeral. citre-peek is classfied to B and its lifetime(?) is controllable.

I actually created citre-peek as an alternative to Eldoc. Eldoc has 2 problems for Citre:

  1. Citre can't accurately capture current function name in all situations, which makes Eldoc unreliable.

  2. Eldoc is typically used to display the signature, but ctags can't generate the signature field for some languages. And, we also need the docstring frequently, and the simplest way to get the docstring without intefering your current writing is peeking the source code.

When doing citre-jump in citre-peek, the overlay is not displayed at the place where I jumped to. It will be nice if the overlay is shown continuously. The person must be escorted always. When jumped at, I would like to see the information about where I jumped from (the information about parent context).

I can imagine this "peek back" design being useful, and this can become a very unique, "Citrish" thing ;)

If what I wrote doesn't contradict your design of citre-peek, I recommend you to give a peculiar name. "escort" is my idea. However, as far as reading dictionary, it has multiple meanings. So I'm not sure it is acceptable or not. Another one is "agent". However, the word becomes not so impressive these days.

It actually contradicts with my design, as descibed above. However, I would like to design an extended version of citre-peek. In fact, let's divide the tools offerd by Citre into 2 categories, one is for code writing and brief code reading, including auto-completion, xref, citre-jump, peek and Eldoc; another is for long code reading, currently including the code map. Their boundary can be simply defined: the latter assumes the code doesn't change, while the former doesn't. Let's put the "escort" in the second category.

edit: I found the "peek back" feature is also useful in brief code reading, and you didn't said you want to use it in long code reading sessions. Let me know your thoughts. If you only need "peek back" as an user option for brief code reading, I can add it to citre-peek.

As for the name, I think "HUD" is actually not bad.

We need to think carefully about "escort":

I would like you to open a new issue to discuss about this new weapon ;)

AmaiKinono commented 4 years ago

citre-peek can list multiple items having the same name. It will be nice if we can sort them.

This is a feature in plan (see the todo file). And, I'm not just thinking about sorting definition locations, but all kinds of candidates Citre could provide. I want to make Citre smart enough to guess which are the candidates that are most possibly need by the user (very likely to be some tags with certain field values), and put them at the beginning. But, I agree that for definition locations, sorting by file (as described in https://github.com/universal-ctags/ctags/pull/2519#issuecomment-617486118) is a basic and must have feature.

AmaiKinono commented 4 years ago

Now the code of citre.el is divided into 3 layers. I wrote this tip in the source code:

To see the outline of this file...run M-x occur with the following query: ^;;;;* \|^(.

Please do this to see how is the code divided into layers. I'll explain a bit here:

  1. The core layer: This focuses on parsing tags files. Once #10 is solved, there won't be any optional argument in functions in this layer. Here are the side effects or environment needed of these functions:

    • citre--get-lines: runs readtags command in the shell.
    • citre--parse-line: relies on the project root, or the pseudo tag _TAG_PROC_CWD after #10 is solved.
    • citre-get-records: calls citre--get-lines and citre--parse-line.
    • citre-get-field: read a file when asking for the line field (the "field" in the code of Citre is a bit different from that in ctags, I may need a better name).

    edit: For citre--parse-line, I think it can get whatever file path that tags file offers, without caring about whether it's absolute or relative, so we don't need to offer any kind of directory to it. Then in citre-get-records, we look into the pseudo tags and expand the paths if they are relative.

    I think it's good enough to test the main APIs: citre-get-field and citre-get-records. They defines the only way that upper components should use to get informations from tags files.

  2. The utils layer. This provides utilities for user tools. Those for finding definitions and auto-completion should be tested, since in the future, many language-specific things would happen here.

  3. The tools layer. This provides tools that the user would directly use. This also includes the whole citre-map.el.

AmaiKinono commented 4 years ago

Close this since we've done the major refactoring and have a test framework now :tada: