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: Add option to prefix generated commit message #113

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ cli(
description: 'Number of messages to generate (Warning: generating multiple costs more) (default: 1)',
alias: 'g',
},
prefix: {
type: String,
description: 'String to prefix to the generated commit message.',
default: '',
},
exclude: {
type: [String],
description: 'Files to exclude from AI analysis',
Expand Down Expand Up @@ -59,6 +64,7 @@ cli(
} else {
aicommits(
argv.flags.generate,
argv.flags.prefix,
argv.flags.exclude,
argv.flags.all,
argv.flags.type,
Expand Down
3 changes: 3 additions & 0 deletions src/commands/aicommits.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { KnownError, handleCliError } from '../utils/error.js';

export default async (
generate: number | undefined,
prefix: string,
excludeFiles: string[],
stageAll: boolean,
commitType: string | undefined,
Expand Down Expand Up @@ -76,6 +77,8 @@ export default async (
let message: string;
if (messages.length === 1) {
[message] = messages;
message = `${prefix} ${message}`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand the benefit of the space, but I'm afraid it becomes restrictive for those that don't want it and will later ask for a CLI flag to disable the space or something. But, I also haven't been able to come out with an example where they wouldn't want the space.

What do you think?

Copy link

Choose a reason for hiding this comment

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

In my case I like the space, because it is usually a ticker number like FOO-123: ${message}. However, if the space is not added, it could be added in the same prefix.

Copy link
Author

@aslakhol aslakhol May 4, 2023

Choose a reason for hiding this comment

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

I considered this and in the end I figured that for the few cases where no space would be needed people could edit the commit after the fact like before.

But on the other hand I don't personally mind removing the space. I use this with an alias that injects the punctuation I need so it would be trivial to include a space in that.

Copy link
Author

Choose a reason for hiding this comment

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

@privatenumber Want me to get rid of the space?


const confirmed = await confirm({
message: `Use this commit message?\n\n ${message}\n`,
});
Expand Down