Clean Django templates to use 4 space indents

[adnrs96]

In this PR we continue the work being done in PR #3658 to clean up the django templates located in
/templates/zerver/ to follow 4 space indentation. The improvements to pretty_print tool has been pushed as a commit to PR #3658. All the files cleaned up in this PR does not pose any merge conflicts with other open PR 's.

[smarx]

Automated message from Dropbox CLA bot

@adnrs96, it looks like you've already signed the Dropbox CLA. Thanks!

[showell]

@timabbott I think this is ready to merge, although you might want to look at markdown_help.html as interesting case of where the pretty printer leaves some of the original dubious formatting intact.

[adnrs96]

I have updated the PR with changes to markdown_help.html and some more.

[timabbott]

I merged a few commits, thanks @adnrs96! Posted some comments on chat.zulip.org on a few issues; I think there's a couple small bugs in the formatter for Jinja2 templates.

[timabbott]

Awesome, thanks for doing the next batch of this @adnrs96!

I merged a bunch more of these; posted a few comments on things that seem like they still need some work...

[timabbott]

This looks wrong

[adnrs96]

yeah, had a look and turns out to be issue with non block django tags.

[timabbott]

placeholder is over-indented here.

[timabbott]

This <td> spacing seems off?

[adnrs96]

That is a manual error. markdown_help actually needs manual editing after running tool.
Will fix it.

[adnrs96]

Thanks for reviewing and merging. I have updated this PR with new commits. It turns out there were some overlooked issue in tooling with respect to django tags which do not have a block structure and end tags indents. Now that is fixed and the PR #3658 has also been updated with commit containing changes in tooling.

[timabbott]

I merged another batch of these, with a few manual changes.

• api.html I can't merge because it has some meaningful whitespace in the code blocks -- e.g. the zuliprc output. I kinda want to just redo those sections in markdown...
• features/apps/integrations/hello I can't merge because there's some active work on them that doesn't have a PR open yet, but will soon.

[adnrs96]

Thank you for this. I am curious about the whitespace and zuliprc stuff you mentioned. I quite don't follow what you meant by zuliprc output?
For the features/apps/integrations/hello I am thinking of having them as well in a exception list when we add automatic checking for 4 space indents.

[adnrs96]

Just added new commits 3 of which have been merged already last time, namely files

• portico.html
• accounts_accept_terms.html
• api_endpoints.html

I am afraid to tell you that we missed out some thing in these files which the tool didn't fixed indentation properly for (was a kind of bug in tool, now resolved and updated in commit here under this PR#3658). The new commits have those updates.

Please review and advice.

[timabbott]

So the features.html and integrations.html changes that Brock was working on are now merged, so you can add commits for those when you get a chance.

[timabbott]

left sidebar has some merge conflicts so you should probably regenerate that one; I merged the rest.

[timabbott]

Probably now's a good time to analyze the files that still need to migrate and which PRs are important to merge for this...

[adnrs96]

Sure let me run the analyser again.

[adnrs96]

Just updated the PR with new commits. Here is a list of PR's which should be cleared to proceed further.

templates/zerver/compose.html PR#3118(+1-2) PR#2721(+1-0)
templates/zerver/navbar.html PR#2379(+1-0) PR#2973(+69-60)
templates/zerver/register.html PR#3007(+3-0)
templates/zerver/keyboard_shortcuts.html PR#2292(+8-0) PR#2854(+4-0)
templates/zerver/index.html PR#2292(+1-0)
templates/zerver/home.html PR#2089(+2-0) PR#3002(+2-0)

Also i wanted to know about api.html and markdown stuff relating to it you talked about.

[timabbott]

#3905 is merged, and I think we shouldn't worry about ultra-old PRs (e.g. <2000) with 1 line of changes.

[adnrs96]

Updated with new commit's. Also cleaned files which are in PR's 1954, 749, 1747. These PR's either had merge conflicts for these files already or very minimal change. Updated the list as well.

[timabbott]

I merged a few. With integrations.html, there's a problem with leading whitespace in the <pre> code blocks (check out what the "basecamp" one looks like)

[adnrs96]

Yes just noticed it. This pre tag has given me some hard time. For the timing i have adjusted the pretty printer to just leave alone the pre tag code wherever it is in the html. Obviously this may lead to ugly looking code but we can't do much of anything until we deploy some kind of JS (just looked around on google) to deal with pre tags and the indents. But this might change the meaning of pre tag itself i believe.
(Updated the PR)

[timabbott]

I think the medium-term plan is to port a lot of that documentation into markdown, so maybe not worth worrying about.

[eeshangarg]

@adnrs96: Thanks for your work on this!

A few more recommendations. This is minor, but for future reference, please include the section of the codebase that your commit affects at the front of the commit message (for example integrations/asana.html: Migrate to Markdown.) Please see our Git Guide and commit message guidelines for more information. Its good to adhere to this because it makes a lot easier for reviewers to browse our Git logs and PRs. :)

And one last thing, if you plan to migrate more integrations' docs to Markdown, please try to wrap it at 80 characters (its easier to read with two files open side by side). There are exceptions to this (e.g. long URLs). :)

@adnrs96, @timabbott: Apart from my comment above, this looks good to me for now! I'm sure our future migration efforts will constitute more iterative improvements to the framework!

Thanks! :)

[eeshangarg]

I feel like this part could be more concise and elegant, and therefore less error prone. :) Here's an example:

if context.get('integrations_dict') is not None:
    integration_dir = None
    if markdown_file_path.endswith('doc.md'):
        integration_dir = markdown_file_path.split('/')[0]
    elif 'integrations' in markdown_file_path.split('/'):
        integration_dir = splitext(basename(markdown_file_path))[0]

    integration = context['integrations_dict'][integration_dir]

    context['integration_name'] = integration.name
    if integration.name == 'bitbucket2':
        context['integration_name'] = 'bitbucket'
        
    context['integration_display_name'] = integration.display_name
    if hasattr(integration, 'url'):
        context['integration_url'] = integration.url[3:]

Also, splitext and basename come from os.path, so make sure to add from os.path import basename, splitext at the top of the file.
The integration.name == 'bitbucket2' conditional will soon be removed in favor of a more general approach, making this even conciser. But its fine for now, because I'm still working on that part of the framework. ;)

One more recommendation, instead of catching a KeyError and doing nothing (using pass), it's cleaner to do something like:

if context['integrations_dict'].get(integration_dir) is not None:
    doSomeStuff()

In my opinion, its always good to keep the amount of code that should be wrapped in a try block to the absolute minimum. (Specificity is good!) :)

[zulipbot]

Heads up @adnrs96, we just merged some commits (latest: 7f15cfe) that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

[timabbott]

I went through these and merged the things that I could. Thanks for doing this @adnrs96 and reviewing this @eeshangarg! I made a few changes:

• I line-wrapped all of your new markdown docs; super long lines should be avoided wherever possible.
• I didn't merge the email changes, since they conflict with Common base template for HTML emails #5258, which will also fix this problem for most of those anyway.

I agree with @eeshangarg that we should clean up the templatetags code soon after this to be more readable.

Since this PR is getting cluttered, I'm going to close it and we can continue on a new one.

Thanks for all your work on this so far @adnrs96 !