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

feat: git log the history of one file only #1172

Merged
merged 19 commits into from
Jul 2, 2021

Conversation

snowyu
Copy link
Contributor

@snowyu snowyu commented Jul 1, 2020

Currently the git.log shows all history of the commits. Miss the commits history of the filepath only.

git log -- [filepath]

@snowyu
Copy link
Contributor Author

snowyu commented Jul 1, 2020

with follow supports now, you should set follow and force both to true.

@snowyu
Copy link
Contributor Author

snowyu commented Jul 2, 2020

The another same pr is snowyu:feat/log-async-iterator, It make code more clear to read for using the async iterator. But I have to add the @babel/plugin-proposal-async-generator-functions plugin.

@billiegoose
Copy link
Member

Regarding: src/utils/resolveFileIdInTree.js

This is tricky to do well without having implemented git's actual algorithm for detecting file renames. Comparing just the file oid and grabbing the first one that matches a top-down breadth-first search seems risky. What about using _walk and grabbing all the blob paths that match oid and only following if there is 1 and exactly 1 match? That way we won't accidentally follow the wrong file.

It ends up doing more CPU/IO work to exhaustively determine the file moved rather than was copied, but _walk is also optimized for concurrency so it might even be faster than the current algorithm in some case.

@snowyu
Copy link
Contributor Author

snowyu commented Jul 12, 2020

Aha, I missed a plurality of the same content exists, thank you pointed out.

The _walk is too special(advanced) to walk GitTree. We need a low-level walkTree which walks GitTree only.

@snowyu
Copy link
Contributor Author

snowyu commented Jul 13, 2020

Ok, I've resolved though comparing all the filepaths with last all filepaths of the same content to determine the correct file name.

@jcubic
Copy link
Contributor

jcubic commented Jun 29, 2021

Hi, I'm a new maintainer and I will be able to merge your PR if you resolve the conflicts in the files.

@snowyu
Copy link
Contributor Author

snowyu commented Jun 29, 2021

Merged.

Copy link
Contributor

@jcubic jcubic left a comment

Choose a reason for hiding this comment

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

Please check the conflict, there was a change:

    if (tips.length === 0) {
      break
    }

was removed.

Also please remove the example git hooks from testing fixtures.

src/api/log.js Show resolved Hide resolved
src/api/log.js Outdated Show resolved Hide resolved
src/api/log.js Outdated Show resolved Hide resolved
@snowyu
Copy link
Contributor Author

snowyu commented Jul 1, 2021

It has changed to this:

https://github.com/snowyu/isomorphic-git/blob/634e7da134f96edd1e5b1f34c116bd024f4e219f/src/commands/log.js#L161

    // Stop the loop if there are no more commit parents
    if (tips.length === 0) {
      endCommit(commit)
      break
    }

@jcubic
Copy link
Contributor

jcubic commented Jul 1, 2021

You added a line to the code that got removed. There are no longer that if (tips.length === 0) { check in the code.

    // Stop the loop if there are no more commit parents
    if (tips.length === 0) {
      endCommit(commit) // this make the conflict because surrounding code got removed
      break
    }

@jcubic
Copy link
Contributor

jcubic commented Jul 1, 2021

So please update your fork with the latest version. And check how you can do the same without that line. Right now the code don't use while (true) { and break anymore.

@snowyu
Copy link
Contributor Author

snowyu commented Jul 1, 2021

Oh, I didn't notice that the mian branch has been updated again. done..

@jcubic
Copy link
Contributor

jcubic commented Jul 1, 2021

Note that {Boolean=} was correct the option is optional. You should not remove that only added default parameter and name in bracket.

@jcubic
Copy link
Contributor

jcubic commented Jul 1, 2021

You've merged origin/main but not updated your fork with upstream/main.

@jcubic
Copy link
Contributor

jcubic commented Jul 1, 2021

Sorry I was mistaken your fork was updated. I need to look closer at your code since the logic of that loop was modified recently.

@jcubic jcubic merged commit 27fc08f into isomorphic-git:main Jul 2, 2021
@isomorphic-git-bot
Copy link
Member

🎉 This PR is included in version 1.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

modesty pushed a commit to modesty/isomorphic-git that referenced this pull request Apr 9, 2024
* feat: log a single file history of commits

* feat: log a single file history of commits with follow supports. resolved isomorphic-git#933

* chore(test): jasmine do not support the rejects.toThrowError

* docs: add @snowyu as a contributor

* fix: log file should not add commit when since time

* chore(test): update fixture to check multi deletions log

* chore(test): refresh test snapshot for multi deletions log

* fix(rename): do not search the empty content as filepath in tree

* fix: can not show all history of multi deletions log

* feat: return all the filepaths of the same content in the tree

* fix: git --follow should work with multi same content file

* test: add test git follow with multi same content files

* test(log-file): remove unused example git hooks

* chore(log-file): fix declaraton document for force and follow options

* chore(log-file): correct the declarayion doc

* chore(log-file): correct the declarayion doc in commands
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants