Test core.editor config immediately by listing it in there

[katrinleinweber]

As suggested in #579.

[munkm]

Hi @katrinleinweber, thank you for both the issue and the PR to resolve it! I like that you've added an additional suggestion of how to pop into an editor to look at the config settings that are set. However, I'm a little hesitant to remove how to check the git config settings from the command line as is done in this PR. I do think it's reasonable to check config settings from the command line with git config --list or git config -l. Also, sometimes we just want to check our config settings but opening an editor to the file somewhat implies that we intend to make a change. I think that this would be a good supplement to listing from the command line, but I don't think we should delete the command-line list.

How would people feel if we modified this pr to have something like:

You can check your settings at any time: 
~~~
$ git config --list	
~~~
{: .language-bash}

You can change your configuration as many times as you want: use the	
same commands to choose another editor or update your email address.

Alternatively, if you're more comfortable using an editor than reentering 
the command-line changes you can have git enter you into a core editor 
to modify the config file that stores your config settings. To do that, 
you'd do:

~~~
$ git config --global --edit
~~~	
{: .language-bash}

[katrinleinweber]

Thanks for your comments @munkm! I tried to stick to the "explain what you would take out to make room for [new concept]"-rule ;-) Opening in the editor effectively:

1. validates the previous config example,
2. display a list as well, and
3. show learners a more convenient way to configure git.

That's IMHO more beneficial than extending the lesson with an explanation of --edit.

[munkm]

Ah! I appreciate your reference to the contribution guidelines! I personally interpret this rule as major expansions of the lesson (e.g. adding whole new sections, adding a completely new command, etc.). In the case of your change here, I see including both versions as a clarification about the git config command which is a concept that has already been introduced. Talking about showing git config options in different ways isn't introducing a new concept, but rather clarifying one that is already introduced in this lesson. Though if other maintainers disagree with my interpretation then perhaps we should update the contribution guidelines to be more clear about this.

I agree with the points that you're making about opening the config file in the editor (though I disagree that it is more convenient for all users.) I think that adding the option to modify the git config in an editor is a helpful addition.

Removing git config --list opens up a few avenues for problems early in the lesson:

• if the core editor isn't set properly then entering into the git config could be problematic. New instructors and learners that are nervous about the command line could be put off quickly
• does't show learners the workflow where you'd want to check config settings in a way that's completely separate from editing a file

I personally think that git config --list is important to include. More often than not, I think that users want to check that a config setting has been set and only modify it if necessary, not default to modifying the file that has them.

[katrinleinweber]

The CG states "material", not "whole new sections. My [new concept] addendum was just an example.

if the core editor isn't set properly then entering into the git config could be problematic.

Exactly that's why it should be tested (nost just viewed) immediately, not remain untested until some advanced operation launches learners into vi(m) or an error message unexpectedly ;-) This is my core argument in this PR.

Assuming they use --list after making a copy-paste or typing error when setting the config, isn't it less likely to be spotted by viewing only.

does't show learners the workflow where you'd want to check config settings in a way that's completely separate from editing a file [...] not default to modifying the file that has them.

Opening a file primarily means viewing its content. That is separate from editing it. Which in turn is separate from saving edits. Which can be undone. So, we're at least 2 steps removed from the risk of learners breaking their git config, while ensuring the discovery & repair of the problems right then & there.

[munkm]

The first sentence from the "What not to contribute" section of the CG that you linked above:

Our lessons already contain more material than we can cover in a typical workshop, so we are usually not looking for more concepts or tools to add to them.

concepts are mentioned in the CG. My point is that you aren't adding a new concept. You're clarifying an existing one. I having both options shown here as a way to look at/access the git config settings still abides by the contribution guidelines.

Opening a file primarily means viewing its content.

This is not true or consistent with the rest of the SWC lesson material. The bash lesson heavily emphasizes using cat <filename> to inspect the contents of a file. We only enter into an editor when we want to change it. We open a file in an editor because we intend to make changes to it.

Assuming they use --list after making a copy-paste or typing error when setting the config, isn't it less likely to be spotted by viewing only.

I don't see how it is any less likely that they will be less likely to see an error in the config settings with --list than if it is opened in an editor. Both should have similar verbosity. But --list ensures that an accidental edit doesn't happen.

[katrinleinweber]

I don't see how it is any less likely that they will be less likely to see an error in the config settings with --list than if it is opened in an editor.

Because if they made an error with the core.editor, it will not open, making copy-paste or typing mistakes immediately obvious.

But --list ensures that an accidental edit doesn't happen.

While also ensuring that core.editor remains untested until the middle of 04-changes. Which is IMHO an episode that should not suffer from risk of distraction, while the risk of showing learners their config in an editable way here is acceptable.

[...] having both options shown here as a way to look at/access the git config settings still abides by the contribution guidelines.

If you're fine with expanding the episode, please feel free to edit my suggestion and merge the combination. I activated the "Allow edits from maintainers" option.

[kekoziar]

I agree that git config --list should remain in the lesson. I have seen config issues where git config --global --edit wouldn't open the file, but examining the settings using git config --list helped identify the problem. I think it's important for new learners to know the basic commands.

I also like the idea of testing the core.editor setting - mostly because I've also seen windows config settings fail during a workshop, and it would have been easier to fix at that time than later in the lesson. The modification @munkm proposed allows testing the core editor, but it needs an addendum or to be reworked. An issue is git config --global --edit shows different output than git config --list. This is because git config --global --edit only shows the configurations added using the --global tag, and not other configuration settings which are either repository or system based. This is where we get into scope creep.

While I don't think adding a simple "let's test the core.editor setting" line is adding a new concept, the explanation behind "let's check and change configuration settings using the core.editor" can start to creep into a new topic.

That said, I think git config --list needs to remain in the lesson because it is a basic command. If git config --global --edit is added, it should be immediately after the core.editor is added and only to test that the core.editor configuration works. Anything beyond that I think will end up being a more advanced topic and out of scope of this lesson.

[katrinleinweber]

Thanks for the feedback and sorry that I didn't find time to incorporate it before. How about this version?

[kekoziar]

Branch needs to be update prior to merging.

[kekoziar]

Also, related to #579

[emcaulay]

I like the discussion here -- good work, everyone, in digging into this lesson and this portion of it!

I'd like to vote for going forward with this revision.

I just got out from being a helper on this lesson and I spent a lot of time on helping with the editor variety. And I think it's a great way to build some facility with the idea of using an editor during your git workflow.

Do I understand correctly that @katrinleinweber needs to update her branch and resubmit for this pull request to proceed?

[katrinleinweber]

…katrinleinweber needs to update her branch and resubmit for this pull request to proceed?

@emcaulay & @kekoziar: If the GitHub checks can't be triggered otherwise, I'm fine with doing so. Since there are no merge conflicts, it wouldn't strictly be necessary, IMHO. (See also my last reply).

[kekoziar]

looks good and renders fine. Thank you everyone for this update!