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

--from-context doesn't include footers #837

Closed
1 task done
muja opened this issue Sep 2, 2024 · 5 comments · Fixed by #920
Closed
1 task done

--from-context doesn't include footers #837

muja opened this issue Sep 2, 2024 · 5 comments · Fixed by #920
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@muja
Copy link

muja commented Sep 2, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Description of the bug

 ERROR git_cliff > Template render error:
Variable `commit.footers` not found in context while rendering 'template'

Steps To Reproduce

You can test with my repository's template at https://github.com/muja/unrar.rs, for example where I use footers.

git cliff --context gives footers, however

git cliff --context | git cliff --context --from-context - has no footers.

Expected behavior

footers are read from context

Screenshots / Logs

No response

Software information

  • Project version: 2.5.0

Additional context

Possibly other values are also missing? I haven't fully compared the contexts

@muja muja added the bug Something isn't working label Sep 2, 2024
@orhun orhun added the good first issue Good for newcomers label Sep 2, 2024
@orhun
Copy link
Owner

orhun commented Sep 2, 2024

Good find! This is due to footers being serialized manually:

commit.serialize_field("footers", &SerializeFooters(self))?;

/// A wrapper to serialize commit footers from an iterator using
/// `Serializer::collect_seq` without having to allocate in order to
/// `collect` the footers into a new to `Vec`.
struct SerializeFooters<'a>(&'a Commit<'a>);
impl Serialize for SerializeFooters<'_> {
fn serialize<S>(
&self,
serializer: S,
) -> std::result::Result<S::Ok, S::Error>
where
S: Serializer,
{
serializer.collect_seq(self.0.footers())
}
}

We simply need to add a Vec<Footer> field to Commit for enabling the deserialization.

I might get to it soon, marking it as good first issue in case someone wants to work on it :)

@muja
Copy link
Author

muja commented Sep 5, 2024

FYI I compared the diffs between git cliff --context and git cliff --context | git cliff --context --from-context -. These are the differences:

  1. missing fields: footers, body, breaking_description, breaking
  2. conventional flipped from true to false

@orhun orhun removed their assignment Sep 15, 2024
@orhun
Copy link
Owner

orhun commented Sep 15, 2024

Thanks for the comparison! It will definitely come in handy while working on the fix.

@dqkqd
Copy link
Contributor

dqkqd commented Oct 10, 2024

@orhun
Hi, I'm trying to fix this issue. Surely, adding footers into Commit is an easy fix for this.

But as @muja mentioned above, body, breaking, and some other fields were missing during reading and writing context. I think a proper fix should address them as well.

Looking at the serialization code, I found that we are trying to convert some fields from conv into Commit. For example, assigning message from conv to Commit, and since they are not identical, we lose data after converting to context.

Maybe we should retain some information so that after writing to context, we can read it back without any loss of data.
message should be the good one, since we can create conv from it. Or we can add another field (e.g raw_message) ?

pub fn into_conventional(mut self) -> Result<Self> {
match ConventionalCommit::parse(Box::leak(
self.message.to_string().into_boxed_str(),
)) {

What do you think?

@orhun
Copy link
Owner

orhun commented Oct 13, 2024

Hey @dqkqd, that sounds reasonable 👍🏼 I'm looking at your PR right now :) Thanks

dqkqd added a commit to dqkqd/git-cliff that referenced this issue Oct 13, 2024
* feat(commit): add `raw_message` to `Commit`

* test(fixtures): add test generate all fields in conventional commits

* test(fixtures): add test do not discard missing fields in conventional
commits when reading from context

* docs(website): add `raw_message` fields to `context.md`
dqkqd added a commit to dqkqd/git-cliff that referenced this issue Oct 13, 2024
* feat(commit): add `raw_message` to `Commit`

* test(fixtures): add test generate all fields in conventional commits

* test(fixtures): add test do not discard missing fields in conventional
commits when reading from context

* docs(website): add `raw_message` fields to `context.md`
dqkqd added a commit to dqkqd/git-cliff that referenced this issue Oct 13, 2024
* feat(commit): add `raw_message` to `Commit`

* test(fixtures): add test generate all fields in conventional commits

* test(fixtures): add test do not discard missing fields in conventional
commits when reading from context

* docs(website): add `raw_message` fields to `context.md`
dqkqd added a commit to dqkqd/git-cliff that referenced this issue Oct 14, 2024
* feat(commit): add `raw_message` to `Commit`

* test(fixtures): add test generate all fields in conventional commits

* test(fixtures): add test do not discard missing fields in conventional
commits when reading from context

* docs(website): add `raw_message` fields to `context.md`
dqkqd added a commit to dqkqd/git-cliff that referenced this issue Oct 14, 2024
* feat(commit): add `raw_message` to `Commit`

* test(fixtures): add test generate all fields in conventional commits

* test(fixtures): add test do not discard missing fields in conventional
commits when reading from context

* docs(website): add `raw_message` fields to `context.md`
dqkqd added a commit to dqkqd/git-cliff that referenced this issue Oct 14, 2024
* feat(commit): add `raw_message` to `Commit`

* test(fixtures): add test generate all fields in conventional commits

* test(fixtures): add test do not discard missing fields in conventional
commits when reading from context

* docs(website): add `raw_message` fields to `context.md`
dqkqd added a commit to dqkqd/git-cliff that referenced this issue Oct 14, 2024
* feat(commit): add `raw_message` to `Commit`

* test(fixtures): add test generate all fields in conventional commits

* test(fixtures): add test do not discard missing fields in conventional
commits when reading from context

* docs(website): add `raw_message` fields to `context.md`
orhun pushed a commit that referenced this issue Oct 17, 2024
* feat(commit): add `raw_message` to `Commit`

* test(fixtures): add test generate all fields in conventional commits

* test(fixtures): add test do not discard missing fields in conventional
commits when reading from context

* docs(website): add `raw_message` fields to `context.md`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants