Skip to content

feat: add context to Chunk #14

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

Merged
merged 1 commit into from
Apr 21, 2023
Merged

Conversation

TheCommieAxolotl
Copy link
Contributor

Adds a Chunk.context entry with the context of the Chunk

e.g.

diff --git a/rename.js b/rename.js
index aa39060..0e05564 100644
--- a/rename.js
+++ b/rename.js
@@ -4 +4,5 @@ function hello() {
   // hello world
+  console.log("hello world");

will result in a chunk with

{
	// ...
	context: "function hello() {",
    // ...
}

@TheCommieAxolotl
Copy link
Contributor Author

@yeonjuan Just in case you missed it.

@yeonjuan
Copy link
Owner

yeonjuan commented Apr 13, 2023

Hi @TheCommieAxolotl , Thanks for the PR. I have a question. Why we add the context to the result?
Could you explain more?

@TheCommieAxolotl
Copy link
Contributor Author

Thanks @yeonjuan, I'm using your package under the hood in an upcoming project of mine and I would like to display the context of diff chunks to the user to help them better understand where their code is. I was thinking about just writing my own parser to run as well as yours but I saw you were using a regex and it would be easy for me to contribute.

@yeonjuan
Copy link
Owner

@TheCommieAxolotl I got it. I think it's a bug. The result should contain "function hello() {" as a UnchangedLine (rather than add context). I'll work on it!

@TheCommieAxolotl
Copy link
Contributor Author

@yeonjuan So you would just add an UnchangedLine to the beginning of the chunk?

@yeonjuan
Copy link
Owner

@TheCommieAxolotl Yes Right, the result should contain all lines!

@TheCommieAxolotl
Copy link
Contributor Author

@yeonjuan I think you may be misunderstanding. The "context" isn't always part of the chunk, sometimes it can refer to a line several hundreds of lines above the current chunk and is just shown for orientation.

@yeonjuan
Copy link
Owner

@TheCommieAxolotl

@yeonjuan I think you may be misunderstanding. The "context" isn't always part of the chunk, sometimes it can refer to a line several hundreds of lines above the current chunk and is just shown for orientation.

Yes right, But I don't think we need new node type. Isn't this just the unchanged line output from the git-diff command?

@TheCommieAxolotl
Copy link
Contributor Author

TheCommieAxolotl commented Apr 20, 2023

@yeonjuan No, it is a separate part of the diff output. If you don't want to add a new entry to the chunk I understand but I think adding it as an UnchangedLine is incorrect behavior.

@yeonjuan
Copy link
Owner

yeonjuan commented Apr 21, 2023

@yeonjuan
Copy link
Owner

@TheCommieAxolotl
You're right. I was misunderstanding, the string following the "@@" is not just representing the Unchaned Line. I'll do some more testing and review, thanks.

@yeonjuan yeonjuan self-requested a review April 21, 2023 14:26
Copy link
Owner

@yeonjuan yeonjuan left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for the PR!

@yeonjuan yeonjuan merged commit 6c64a1e into yeonjuan:main Apr 21, 2023
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