Ticket #764 (closed bug: fixed)

Opened 6 years ago

Last modified 5 years ago

t/codingstd/c_indent.t needs to handle indents after #ifdef better

Reported by: jkeenan Owned by: jkeenan
Priority: minor Milestone:
Component: coding_standards Version: 1.2.0
Severity: medium Keywords: codingstd indent
Cc: Language:
Patch status: applied Platform:

Description

The following was part of a modification made to src/pmc/filehandle.pmc today:

Index: src/pmc/filehandle.pmc
===================================================================
--- src/pmc/filehandle.pmc      (revision 39541)
+++ src/pmc/filehandle.pmc      (working copy)
@@ -359,7 +359,7 @@
     METHOD readline_interactive(STRING *prompt :optional, INTVAL got_prompt :opt_flag) {
         STRING *string_result = NULL;
 #ifdef PARROT_HAS_READLINE
-    char * const r = readline(got_prompt ? prompt->strstart : NULL);
+        char * const r = readline(got_prompt ? prompt->strstart : NULL);

The indent was increased from 4 to 8 to be consistent with all the other indentation in the file.

Unfortunately, this sparks a failure in t/codingstd/c_indent.t.

$ perl t/codingstd/c_indent.t src/pmc/filehandle.pmc
1..2
ok 1 - Correctly indented preprocessor directives
not ok 2 - Correctly indented C files
#   Failed test 'Correctly indented C files'
#   at t/codingstd/c_indent.t line 221.
# incorrect indenting in C file found 1 occurrences in 1 files:
# src/pmc/filehandle.pmc:362
#     apparent non-4 space indenting (8 spaces)
# Looks like you failed 1 test of 2.

Now, this is a case where the limitation of our test file is the cause of the problem. We need to fix t/codingstd/c_indent.t to handle cases like this.

Since we release tomorrow, we need a hack for the source code file. This works and was reviewed by Coke on #parrot.

Index: src/pmc/filehandle.pmc
===================================================================
--- src/pmc/filehandle.pmc	(revision 39578)
+++ src/pmc/filehandle.pmc	(working copy)
@@ -359,6 +359,7 @@
     METHOD readline_interactive(STRING *prompt :optional, INTVAL got_prompt :opt_flag) {
         STRING *string_result = NULL;
 #ifdef PARROT_HAS_READLINE
+    /* 4-column indent to get c_indent.t to DTRT */
         char * const r = readline(got_prompt ? prompt->strstart : NULL);
 
         if (r) {

kid51

Change History

  Changed 6 years ago by jkeenan

Bandaid applied in r39579.

  Changed 6 years ago by jkeenan

  • owner set to jkeenan
  • status changed from new to assigned

Creating cindent branch in SVN to work on this ticket. Anyone with insight is welcome to help.

kid51

  Changed 6 years ago by jkeenan

  • component changed from none to coding_standards

in reply to: ↑ description ; follow-up: ↓ 5   Changed 6 years ago by jkeenan

  • patch changed from new to applied

Replying to jkeenan:

A partial solution was implemented in r39642.

Partial -- because I had to change a not-necessarily-incorrect indentation in one other file in order to get the revision that worked on src/pmc/filehandle.pmc to work on all files.

However, in the course of this work I did a lot of code cleanup of the PBP nature on t/codingstd/c_indent.t. So I think on balance we're ahead of the game.

kid51

in reply to: ↑ 4   Changed 5 years ago by jkeenan

  • status changed from assigned to closed
  • resolution set to fixed

Replying to jkeenan:

Replying to jkeenan: A partial solution was implemented in r39642.

There has been no objection to this solution, so I think we can close this ticket. In r39889 I added some inline comments documenting internal subroutine dump_state() which you can use to debug C indentation problems.

kid51

Note: See TracTickets for help on using tickets.