Add support to skip prompting filled info #536

momsse · 2021-02-05T17:57:17Z

Description

Add support to an option that will skip asking gitmoji/title/message when they are filled.

Issue: #534

Tests

All tests passed.

codecov · 2021-02-05T17:58:18Z

Codecov Report

Merging #536 (6771aeb) into master (9c8ba88) will increase coverage by 2.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #536      +/-   ##
==========================================
+ Coverage   90.17%   92.19%   +2.02%     
==========================================
  Files          22       23       +1     
  Lines         173      205      +32     
  Branches       25       35      +10     
==========================================
+ Hits          156      189      +33     
  Misses         14       14              
+ Partials        3        2       -1

Impacted Files	Coverage Δ
src/commands/config/prompts.js	`100.00% <ø> (ø)`
src/commands/commit/index.js	`100.00% <100.00%> (ø)`
src/commands/commit/prompts.js	`80.00% <100.00%> (+22.85%)`	⬆️
src/commands/config/index.js	`100.00% <100.00%> (ø)`
src/utils/configurationVault.js	`100.00% <100.00%> (ø)`
src/utils/getDefaultAnswers.js	`100.00% <100.00%> (ø)`
src/utils/getDefaultCommitContent.js	`100.00% <100.00%> (ø)`
src/commands/commit/guard.js	`100.00% <0.00%> (+16.66%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9c8ba88...6771aeb. Read the comment docs.

carloscuesta

Thanks for sending a PR!

Overall code is a little bit complex just pointed out some questions and comments

carloscuesta · 2021-02-07T16:43:56Z

src/commands/commit/prompts.js


-  return [
-    {
+  if (!defaultAnswers.gitmoji) {


Can we stick to the old format? I just want to keep immutability in the codebase as much as possible 🙏🏼

carloscuesta · 2021-02-07T16:45:15Z

src/commands/config/prompts.js

+  },
+  {
+    name: CONFIGURATION_PROMPT_NAMES.SKIP_PROMPTING_FILLED_INFO,
+    message: 'Skip prompting already filled info',


I think we need to clarify better what this option is doing. For you this might be already familiar but for someone who uses the cli and doesn't looks into the PR issues I don't think Skip prompting already filled info is an understandable message, what information is referring this to ?

Sure perhaps a more verbose sentence of what it does ?

Skip prompting for gitmoji, title and message commit when retrieved

carloscuesta · 2021-02-07T16:45:33Z

src/commands/config/prompts.js

@@ -5,7 +5,8 @@ export const CONFIGURATION_PROMPT_NAMES = {
  AUTO_ADD: 'autoAdd',
  EMOJI_FORMAT: 'emojiFormat',
  SCOPE_PROMPT: 'scopePrompt',
-  SIGNED_COMMIT: 'signedCommit'
+  SIGNED_COMMIT: 'signedCommit',
+  SKIP_PROMPTING_FILLED_INFO: 'skipPromptingFilledInfo'


Same as for this option, is a little bit ambiguous

Right, do you have some suggestion ?

carloscuesta · 2021-02-07T16:49:26Z

src/utils/getDefaultCommitContent.js

+  let gitmoji = null
+  let title = commitFileContent[COMMIT_TITLE_LINE_INDEX]
+
+  const gitmojiRegexResult =
+    configurationVault.getEmojiFormat() === EMOJI_COMMIT_FORMATS.CODE
+      ? gitmojiCodeRegex.exec(title)
+      : gitmojiSymbolRegex.exec(title)
+  if (gitmojiRegexResult && gitmojiRegexResult.length === 3) {
+    ;[, gitmoji, title] = gitmojiRegexResult
+  }


Overall this is fairly complex, there are some magic numbers can you explain what are you trying to do in this block?

Basically extracting the gitmoji from the title (3 is the size of the regex array when it succeed).

I can extract the logic to his own function to make it easier to read. Is it fair enough ?

carloscuesta · 2021-02-07T16:49:35Z

src/utils/getDefaultCommitContent.js

+const gitmojiSymbolRegex = /^(\p{Emoji_Presentation})\s*(.*)/gu
+const gitmojiCodeRegex = /^(:[a-zA-Z_]+:)\s*(.*)/gu


Why do we need this?

To skip the gitmoji prompt I have to detect if the user has already filled it on the title part of the commit. This is the regex to do the job.

carloscuesta · 2021-02-07T16:52:28Z

src/commands/commit/prompts.js

-import getDefaultCommitContent from '../../utils/getDefaultCommitContent'
-import { type CommitMode } from './index'


Why we moved this outside of this module?

Because I need the result of the function call for getDefaultAnswers and prompts. To avoid call it twice I moved it down to commit/index

carloscuesta · 2021-02-07T16:53:55Z

src/commands/commit/index.js

+  const commitContent = getDefaultCommitContent(mode)
+  const defaultAnswers = getDefaultAnswers(commitContent)


Why we moved this at commit level?

Same answer as above.

momsse · 2021-02-07T21:53:32Z

To resume:

getDefaultCommitContent will also extract gitmoji from the commit message.
All information retrieved are used by getDefaultAnswers
Those default answers are passed to the prompts to avoid asking the user information that we retrieved

If all those change are enough to include this feature I will to make them next week. If you judge the logic too complex (and it is) to be integrated in the project I completely understand and will pass.

Thanks you for your time reviewing it 🙏

carloscuesta · 2021-02-08T15:42:38Z

I think I've got an idea to simplify the implementation of this! 😊

Taking a look at the Inquirer.js docs you can see there's a when option that could be configured for every prompt. What this property does?

when: (Function, Boolean) Receive the current user answers hash and should return true or false depending on whether or not this question should be asked. The value can also be a simple boolean.

So basically, what we can try, is to add the when property to the title and message prompts conditionally based on the configurationVault option. Something like:

{
      name: 'title',
      message: 'Enter the commit title',
      validate: guard.title,
      transformer: (input: string) => {
        return `[${
          (title || input).length
        }/${TITLE_MAX_LENGTH_COUNT}]: ${input}`
      },
      ...(title ? { default: title } : {}),
      ...(configurationVault.skipOption ? { when: true } : {})
    },
    {
      name: 'message',
      message: 'Enter the commit message:',
      validate: guard.message,
      ...(message ? { default: message } : {}),
      ...(configurationVault.skipOption ? { when: true } : {})
    }

Then we can unify all the prompts logic inside of prompts.js

What do you think?

momsse · 2021-02-08T21:39:13Z

Hi @carloscuesta,

Thanks for the suggestion. This can simplify the prompt a little bit (we do not need to read defaultAnswers). Unfortunately when we use this option { when: false } the question is skipped but the default answer too :/ This is not a solution that can help us removing getDefaultAnswers function.

Tell me if I missed the point. Here the changes I tried:

export default (
  gitmojis: Array<Gitmoji>,
  { gitmoji, title, message }: CommitContent
): Array<Object> => {
  const skipPromptingFilledInfo = configurationVault.getSkipPromptingFilledInfo()

  return [
    {
      name: 'gitmoji',
      message: 'Choose a gitmoji:',
      type: 'autocomplete',
      source: (answersSoFor: any, input: string) => {
        return Promise.resolve(
          filterGitmojis(input, gitmojis).map((gitmoji) => ({
            name: `${gitmoji.emoji}  - ${gitmoji.description}`,
            value: gitmoji[configurationVault.getEmojiFormat()]
          }))
        )
      },
      ...(gitmoji ? { default: gitmoji } : {}),
      ...(skipPromptingFilledInfo && gitmoji ? { when: false } : {}),
    },
    {
      name: 'scope',
      message: 'Enter the scope of current changes:',
      validate: guard.scope,
      ...(!configurationVault.getScopePrompt() ? { when: false } : {}),
    },
    {
      name: 'title',
      message: 'Enter the commit title',
      validate: guard.title,
      transformer: (input: string) => {
        return `[${
          (title || input).length
        }/${TITLE_MAX_LENGTH_COUNT}]: ${input}`
      },
      ...(title ? { default: title } : {}),
      ...(skipPromptingFilledInfo && title && guard.title(title) ? { when: false } : {}),
    },
    {
      name: 'message',
      message: 'Enter the commit message:',
      validate: guard.message,
      ...(message ? { default: message } : {}),
      ...(skipPromptingFilledInfo && message && guard.message(message) ? { when: false } : {}),
    }
  ]
}

carloscuesta · 2021-02-13T22:09:56Z

Will take a look at this shortly!

carloscuesta · 2021-02-27T20:56:00Z

Hey @momsse sorry for the delay!

Since we don't have a simple way to introduce this feature at the moment, I think it would be great to skip at this moment (because it's not a big deal for the UX of the cli). Hope we can revisit this on a future iteration and opportunity

Thanks for the time and effort you put on this PR, I really appreciate it, thanks ❤️

Karsens · 2021-12-12T02:54:36Z

Hi, I saw that this wasn't merged. Very unfortunate because I really need this as I usually don't put so much effort into my git commits and would really like them fast. So I won't use gitmoji for now. :( Hope you can merge this in the future!

✨ add support to skip prompting filled info

d0e0c8a

momsse force-pushed the skip-prompting-filled-info branch from 6555b50 to d0e0c8a Compare February 5, 2021 17:58

✅ fix missing awaits on async function

6771aeb

carloscuesta reviewed Feb 7, 2021

View reviewed changes

carloscuesta closed this Feb 27, 2021

github-actions bot locked as resolved and limited conversation to collaborators Feb 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support to skip prompting filled info #536

Add support to skip prompting filled info #536

momsse commented Feb 5, 2021 •

edited

Loading

codecov bot commented Feb 5, 2021 •

edited

Loading

carloscuesta left a comment

carloscuesta Feb 7, 2021

momsse Feb 7, 2021

carloscuesta Feb 7, 2021

momsse Feb 7, 2021

carloscuesta Feb 7, 2021

momsse Feb 7, 2021

carloscuesta Feb 7, 2021

momsse Feb 7, 2021

carloscuesta Feb 7, 2021

momsse Feb 7, 2021

carloscuesta Feb 7, 2021

momsse Feb 7, 2021

carloscuesta Feb 7, 2021

momsse Feb 7, 2021

momsse commented Feb 7, 2021 •

edited

Loading

carloscuesta commented Feb 8, 2021 •

edited

Loading

momsse commented Feb 8, 2021

carloscuesta commented Feb 13, 2021

carloscuesta commented Feb 27, 2021

Karsens commented Dec 12, 2021

		const gitmojiSymbolRegex = /^(\p{Emoji_Presentation})\s(.)/gu
		const gitmojiCodeRegex = /^(:[a-zA-Z_]+:)\s(.)/gu

		import getDefaultCommitContent from '../../utils/getDefaultCommitContent'
		import { type CommitMode } from './index'

		const commitContent = getDefaultCommitContent(mode)
		const defaultAnswers = getDefaultAnswers(commitContent)

Add support to skip prompting filled info #536

Add support to skip prompting filled info #536

Conversation

momsse commented Feb 5, 2021 • edited Loading

Description

Tests

codecov bot commented Feb 5, 2021 • edited Loading

Codecov Report

carloscuesta left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

momsse commented Feb 7, 2021 • edited Loading

carloscuesta commented Feb 8, 2021 • edited Loading

momsse commented Feb 8, 2021

carloscuesta commented Feb 13, 2021

carloscuesta commented Feb 27, 2021

Karsens commented Dec 12, 2021

momsse commented Feb 5, 2021 •

edited

Loading

codecov bot commented Feb 5, 2021 •

edited

Loading

momsse commented Feb 7, 2021 •

edited

Loading

carloscuesta commented Feb 8, 2021 •

edited

Loading