Bogdan-Lyashenko / codecrumbs

Learn, design or document codebase by putting breadcrumbs in source code. Live updates, multi-language support and more.
https://codecrumbs.io
BSD 3-Clause "New" or "Revised" License
2.71k stars 101 forks source link

Multiple crumbs in the same file #3

Closed andrewmherren closed 5 years ago

andrewmherren commented 5 years ago

It seems that currently only one crumb per file is supported, however it would be incredibly useful if there was a way to do multiple crumbs.

The particular use case I am thinking of is a file (like a reducer) that might be involved in more than one trail.

PS Thanks for your work on this package, its a brilliant idea!

Bogdan-Lyashenko commented 5 years ago

@andrewmherren can you please share the code snipet how did you try it? It should support that, maybe a bug with parser for you particular case, need to see code to say more ;) Thanks đź‘Ť I tested on "single codecrumb"


//cc:one
export const AuthState = new Record({
  authenticated: false,
  id: null
});

//cc:two
export function authReducer(state = new AuthState(), {payload, type}) {
  switch (type) {
    //cc:three

image

And with "trail"

      return state.merge({
        authenticated: !!payload, // cc:signin#3;toggle 'authenticated' flag
        id: payload ? payload.uid : null
      });

    case SIGN_OUT_SUCCESS:
      //cc:signin#2;sign out

image

I agree though, order of trailed crumbs inside one file can be messed up, need to fix that.

andrewmherren commented 5 years ago

@Bogdan-Lyashenko Interesting, I played with this some more and I seem to be getting intermittent behavior that is making things a little confusing. I documented what I tried and what the results were below to give you an idea of what i'm seeing but i'm not sure how actionable this is yet

I will try to put in some more time investigating this in the next couple days and see if I can come up with anything repeatable or if I can recreate anything similar in the demo project. I will also try a couple of other projects to see if maybe there is something special about this one in particular.

I started with this fairly simple component in my project and added a couple of trail beginnings and a regular (non trail) crumb. Its worth noting that I tried to setup the simplest test case I could but by having trails with only one point i'm probably hitting on a somewhat unexpected use case so maybe that is part of the problem.

image

When viewed in the browser it showed only the second trail:

image

after changing nothing but reloading http://localhost:2018 the trail menu changed slightly and the crumb display showed the step instead of the line number

image

I then noticed that some of your comments had trailing semicolons in the example and thought maybe that was effecting parsing so I edited the code to this:

image

and code crumbs auto updated (without refresh) to now add the -no trail- option but trail1 still wasnt showing.

image

Next I removed the semicolons again and tried removing my comment block at the top

image

This gave tail1 and trail2 options but no opetion for -no trail-

image

Next I changed nothing and reloaded the browser but there was no change. Then I tried adding the comment back in (just ctrl-z, ctrl-s)

image

and this time it continues to show both trails but no option for -no trail-

image

As I said, I'll see if I can do some more testing in the next couple of days and hopefully find more useful clues

Bogdan-Lyashenko commented 5 years ago

@andrewmherren thanks for your investigation, I will double check that as well. From known issues, I remember that codecrumb near JSX can be missed, I need to fix that. However, there is no functionality to display "multiple trails" together right now. Like from your code: //cc:trail1 and //cc:trail2 are two separate trails, so you can only select one of them to be displayed (from that dropdown in top bar)

andrewmherren commented 5 years ago

@Bogdan-Lyashenko Yes, I understand about only displaying one trail at a time and I totally see why you made that decision.

I spent a little more time on this and I think I have found a pattern that may be useful. I think the issues I was having before were a combination of what you mentioned about crumbs near JSX but also what I will describe below.

It seems that maybe there is an issue around finding the end of a crumb. I found that if a crumb is followed by a comment (// or /**/ syntax) without at least one line of code between, the curmp will be missed. Here are a few examples using the demo project to better explain what I mean:

If i add a comment after the crumb in exmaple-project/src/auth/actions.js like this:

image

or like this:

image

Then the associated step 1 crumb for the signin trail is no longer parsed

image

However a comment added before the crumb works just fine:

image

result:

image

Additionally, having at least one line of code between the crumb a comment works just fine:

image

crumb displays correctly:

image

Lastly, all of the above conditions apply if the rather than a comment we add a second crumb

image

then the first crumb will be ignored but the second one will be displayed:

image

If the order of the crumbs is swapped:

image

which ever one comes last is displayed:

image

And again, if there is at least one line of code between the crumbs:

image

then both are properly displayed

image

Now that I understand this it is easy to work around but it would be nice to be able to do something like:

//cc:signin#1;firebase sign in
//A detailed explanation about how this step of the trail works
//since the crumb name needs to be pretty short but the crumbs view
//in the sidebar will take people straight to the full comment

Anyway, I hope that is helpful and thank you again for your work on this amazing project. I am really excited to start using this in all of my projects!

Bogdan-Lyashenko commented 5 years ago

@andrewmherren I see.. yeah, we need to fix that. Wanna give it a try? I can point out where to look in code. I am busy right now with "multiple codebases support" feature, wanna finish it first without switching context. Thanks.

andrewmherren commented 5 years ago

@Bogdan-Lyashenko I'd love to, I did a bit of digging around and found an existing ToDo comment that seems to resolve this issue so I created PR #11

I'm new to contributing so i'm definitely up to any feedback you have! Thanks

Bogdan-Lyashenko commented 5 years ago

Answered in PR (and merged it ;)

andrewmherren commented 5 years ago

Thanks! I'll do a new PR implementing your loop suggestions this weekend if not sooner.

While doing this, I also noticed that the comments array includes both CommentLines //style comments and CommentBlocks /* style comments */. Currently code crumbs have to exist in CommentLines but i'm trying to think of a good way to take advantage of them in blocks as well. I'll probably submit a new issue for that in the next few days to get your input and maybe bounce some ideas back and forth if you're interested in adding support for that.

Bogdan-Lyashenko commented 5 years ago

Can your hold the thing with “block comment” for a while, I will need to write “stupid fallback parser” for other languages (like ruby, where I don’t have AST parser) first, so let’s keep it simple and then iterate. But you may close this issue (since you fixed it) and create one more for “block comments” - simply to not forget about that

andrewmherren commented 5 years ago

Ah yes, I can see how that will get tricky. Ok, will do. Thanks!